Release/0.15 (Issue #35) #41
Conversation
… add tests (fixes dbpedia#19)
… add tests and docs note (fixes dbpedia#19)
📝 WalkthroughWalkthroughRelease preparation for version 0.15 with Vault-based authentication for protected Databus hosts, comprehensive docstring coverage enhancements, improved error handling for download authentication failures, new tests validating Vault auth behavior, and CLI refactoring to encapsulate main entry point. Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant DL as download.py
participant Vault as Vault Service
participant Host as Protected Host
User->>DL: api_download(url, vault_token_file)
alt Host in VAULT_REQUIRED_HOSTS
DL->>DL: Check host requires Vault
alt vault_token_file missing
DL-->>User: DownloadAuthError<br/>"Vault token required"
else vault_token_file provided
DL->>Host: HEAD request (initial check)
Host-->>DL: 401 Unauthorized
DL->>DL: Read vault_token from file
DL->>Vault: POST token exchange<br/>(host-specific)
Vault-->>DL: Access token
DL->>Host: Retry HEAD/GET with<br/>Authorization: Bearer
alt Success
Host-->>DL: File content/metadata
DL-->>User: Download complete
else 401/403 after retry
DL-->>User: DownloadAuthError<br/>"Token invalid/expired" or<br/>"Insufficient permissions"
end
end
else Non-protected host
DL->>Host: HEAD request
alt 401 without Bearer
DL-->>User: DownloadAuthError<br/>"API key required"
else Success
Host-->>DL: Content
DL-->>User: Download complete
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/conftest.py (1)
1-30: LGTM!A lightweight SPARQLWrapper shim for tests when the package isn't installed. The conditional guard at line 5 prevents overwriting a real installation.
One optional enhancement: if future tests need different SPARQL responses, consider making
DummySPARQLconfigurable or adding a pytest fixture that can customize the return value.databusclient/api/delete.py (1)
41-48: Minor docstring/type inconsistency.The type hint specifies
List[str]but the docstring says "Iterable of full Databus URIs". Consider aligning them for consistency.🔎 Option 1 - Update docstring to match type hint:
def add_uris(self, databusURIs: List[str]): """Add multiple Databus URIs to the deletion queue. Args: - databusURIs: Iterable of full Databus URIs. + databusURIs: List of full Databus URIs. """🔎 Option 2 - Update type hint to match docstring (more flexible):
+from typing import Iterable, List + - def add_uris(self, databusURIs: List[str]): + def add_uris(self, databusURIs: Iterable[str]):
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
CHANGELOG.md(1 hunks)README.md(2 hunks)databusclient/__init__.py(1 hunks)databusclient/__main__.py(1 hunks)databusclient/api/delete.py(2 hunks)databusclient/api/deploy.py(7 hunks)databusclient/api/download.py(4 hunks)databusclient/api/utils.py(2 hunks)databusclient/cli.py(2 hunks)databusclient/extensions/webdav.py(4 hunks)pyproject.toml(1 hunks)tests/conftest.py(1 hunks)tests/test_download_auth.py(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
13-13: Multiple headings with the same content
(MD024, no-duplicate-heading)
🔇 Additional comments (28)
databusclient/extensions/webdav.py (4)
1-7: Module docstring is clear and informative. Provides a concise summary of the module's purpose (WebDAV/Nextcloud upload helper), describes the workflow (SHA-256 checksums, rclone copy, metadata generation), and references key functions. Good foundation for developer orientation.
16-24: Docstring forcompute_sha256_and_lengthis well-structured. Args and Returns are clearly documented; naming conventions match the actual return values (hexdigest()andtotal_length). Makes the function's contract explicit.
37-42: Docstring forget_all_filesclearly distinguishes file vs. directory behavior. The description of returning absolute paths for recursive directory walks is accurate and helpful for callers. Good level of detail without verbosity.
52-65: Docstring forupload_to_webdavis comprehensive and well-documented. All parameters are clearly described, and the return value includes the dict keys (filename,checksum,size,url), making it easy for downstream consumers likedeploy_from_metadatato understand the data structure. Excellent documentation addition.pyproject.toml (1)
3-3: LGTM!Version bump to 0.15 aligns with the release preparation documented in CHANGELOG.md and README.md.
README.md (2)
44-48: LGTM!Clear upgrade instructions for users who may have an older version installed. The explicit version pin
databusclient==0.15ensures users get the intended release.
173-174: LGTM!Helpful clarification about when Vault tokens are required. This aligns with the Vault authentication improvements mentioned in the changelog.
databusclient/__main__.py (1)
9-19: LGTM!Encapsulating
cli.app()in a namedmain()function with a__name__ == "__main__"guard is a good practice that improves testability and prevents side effects on import.databusclient/__init__.py (1)
1-21: LGTM!Good docstring additions that improve documentation coverage. The module and function docstrings clearly explain the package's purpose and the
run()function's role.databusclient/api/delete.py (1)
1-70: LGTM!Comprehensive docstrings added to the
DeleteQueueclass and its methods. The documentation clearly explains the purpose and usage of each method.databusclient/api/utils.py (2)
1-5: LGTM! Clear module documentation.The module docstring provides a helpful overview of the utility functions and their usage across API submodules.
33-54: LGTM! Improved docstring clarity.The updated docstrings are more concise and use consistent formatting. The descriptions accurately reflect the function behaviors.
databusclient/api/download.py (5)
16-25: LGTM! Well-designed authentication constants.The explicit host whitelist (VAULT_REQUIRED_HOSTS) and dedicated exception class (DownloadAuthError) provide clear security boundaries and enable user-friendly error handling. The comment clearly marks this as the authoritative source.
69-77: LGTM! Effective fail-fast validation.The early hostname check prevents confusing downstream authentication errors and provides clear guidance to users. Using
urlparsefor hostname extraction is the correct approach.
82-84: LGTM! Consistent error handling.Converting to DownloadAuthError provides consistent, user-friendly error reporting for authentication failures.
110-143: LGTM! Secure and well-structured Vault authentication flow.The authentication logic correctly:
- Restricts token exchange to explicitly configured hosts (VAULT_REQUIRED_HOSTS)
- Provides clear, actionable error messages for auth failures
- Maps HTTP status codes to user-friendly explanations
The removal of
Accept-Encodingat line 134 likely avoids compression-related issues during retry, though it's worth confirming this is intentional behavior.
146-153: LGTM! Comprehensive error coverage.The generic 403/401 handlers provide helpful fallback messages for authentication failures outside the Bearer token flow, guiding users to check their credentials.
databusclient/api/deploy.py (3)
1-6: LGTM! Clear module documentation.The module docstring provides a helpful overview of the deploy functionality and its main components.
35-199: LGTM! Enhanced function documentation.The updated docstrings across multiple helper functions provide clear, concise descriptions of behavior, parameters, and return values. The documentation improvements aid maintainability without changing functionality.
277-296: LGTM! Robust input validation.The validation logic correctly:
- Checks for missing required keys and provides clear error messages
- Validates that size is a positive integer
- Validates SHA-256 checksums are exactly 64 hexadecimal characters
This defensive validation catches malformed metadata early and provides helpful error messages.
databusclient/cli.py (3)
10-10: LGTM! Required import for error handling.The DownloadAuthError import enables the CLI to provide user-friendly error messages for authentication failures.
16-20: LGTM! Improved CLI documentation.The enhanced docstring provides a clear overview of available commands.
178-190: LGTM! Clean error handling integration.The try/except block correctly catches DownloadAuthError and converts it to a ClickException, ensuring authentication errors are presented to users in a clear, CLI-friendly format. The original error message is preserved for maximum clarity.
tests/test_download_auth.py (5)
12-32: LGTM! Well-designed test helper.The
make_responsehelper effectively mocks the requests.Response interface, includingiter_contentfor streaming andraise_for_statusfor error handling. This provides clean test setup across multiple test cases.
35-42: LGTM! Effective test for early validation.The test correctly verifies that attempting to download from a Vault-required host without a token raises DownloadAuthError with an appropriate message.
45-55: LGTM! Validates normal download path.The test confirms that downloads from non-Vault hosts proceed without requiring a Vault token, ensuring the authentication restrictions are properly scoped.
58-87: LGTM! Comprehensive token exchange error handling test.The test correctly simulates the entire token exchange flow and verifies that a 401 response after token exchange produces a clear error message about token validity. The use of
monkeypatchfor REFRESH_TOKEN avoids filesystem dependencies.
90-104: LGTM! Validates permission error handling.The test confirms that 403 Forbidden responses produce clear error messages about insufficient permissions, providing good coverage of authorization failure scenarios.
| # Changelog | ||
|
|
||
| ## 0.15 - Prepared release | ||
|
|
||
| - Prepare PyPI release 0.15. | ||
| - Restrict Vault token exchange to known hosts and provide clearer auth errors. | ||
| - Add tests for Vault auth behavior. | ||
| - Documentation: note about Vault-hosts and `--vault-token` usage. | ||
|
|
||
| (See PR and issue tracker for details.) |
There was a problem hiding this comment.
Remove duplicate changelog section.
Lines 13-22 duplicate the header and 0.15 release notes from lines 1-12. This appears to be accidental content duplication.
🔎 Apply this diff to remove the duplicate content:
Note: After merging this branch, publish a PyPI release (version 0.15) so
`pip install databusclient` reflects the updated CLI behavior and bug fixes.
-# Changelog
-
-## 0.15 - Prepared release
-
-- Prepare PyPI release 0.15.
-- Restrict Vault token exchange to known hosts and provide clearer auth errors.
-- Add tests for Vault auth behavior.
-- Documentation: note about Vault-hosts and `--vault-token` usage.
-
-(See PR and issue tracker for details.)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Changelog | |
| ## 0.15 - Prepared release | |
| - Prepare PyPI release 0.15. | |
| - Restrict Vault token exchange to known hosts and provide clearer auth errors. | |
| - Add tests for Vault auth behavior. | |
| - Documentation: note about Vault-hosts and `--vault-token` usage. | |
| (See PR and issue tracker for details.) | |
| # Changelog | |
| ## 0.15 - Prepared release | |
| - Prepare PyPI release 0.15. | |
| - Restrict Vault token exchange to known hosts and provide clearer auth errors. | |
| - Add tests for Vault auth behavior. | |
| - Documentation: note about Vault-hosts and `--vault-token` usage. | |
| (See PR and issue tracker for details.) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
13-13: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 Prompt for AI Agents
In CHANGELOG.md around lines 13 to 22, the block repeating the header and "##
0.15 - Prepared release" release notes is a duplicate of lines 1-12; remove the
redundant lines 13–22 so the changelog contains only a single copy of the 0.15
entry. Ensure no other content or spacing is altered beyond deleting that
duplicate block and that the file ends with a single consolidated changelog
section.
1 participant
Pull Request
Description
Bump and publish a new PyPI release so the pip-installed package matches the repository (fix CLI usage mismatch). This PR updates package metadata to 0.15, adds a changelog entry, and updates README installation/CLI notes so pip install databusclient yields the documented behavior. It prepares the repo for a PyPI release (tagging instructions and release notes included).
Related Issues
Issue #35
Type of change
Checklist:
poetry run pytest- all tests passedpoetry run ruff check- no linting errorsSummary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.