# PR: Release databusclient 0.15 to PyPI (Issue #35) #41
Conversation
… add tests (fixes dbpedia#19)
… add tests and docs note (fixes dbpedia#19)
📝 WalkthroughWalkthroughRelease 0.15: version bump and release docs; widespread docstring additions; minor runtime changes — DeleteQueue.batch deletions now force without confirmation, CLI maps DownloadAuthError to ClickException, and entrypoint refactored to a main() function. Changes
Sequence Diagram(s)(omitted — changes do not introduce a new multi-component control flow requiring visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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.
DhanashreePetare
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
databusclient/api/delete.py (2)
59-67: Critical: Hardcodedforce=Truebypasses user confirmation for batch deletions.The
execute()method always passesforce=Trueto_delete_list, which bypasses confirmation prompts even when the user explicitly callsdelete(..., force=False). Since deletions are irreversible (line 84), this creates a safety risk.Root cause:
DeleteQueueonly storesdatabus_keybut not the originalforceparameter from thedelete()call, so there's no way to preserve the user's confirmation preference.Impact: Users expecting confirmation prompts for destructive operations won't receive them when using the queue.
🔎 Proposed fix: Preserve force parameter in DeleteQueue
Modify
DeleteQueueto preserve theforceparameter:class DeleteQueue: """ A queue to manage multiple Databus resource deletions. Allows adding multiple databus URIs to a queue and executing their deletion in batch. """ - def __init__(self, databus_key: str): + def __init__(self, databus_key: str, force: bool = False): """Create a DeleteQueue bound to a given Databus API key. Args: databus_key: API key used to authenticate deletion requests. + force: If True, skip confirmation prompts during execution. """ self.databus_key = databus_key + self.force = force self.queue: set[str] = set()Then update
execute()to use the stored value:def execute(self): """Execute all queued deletions. Each queued URI will be deleted using `_delete_resource`. """ _delete_list( list(self.queue), self.databus_key, - force=True, + force=self.force, )Finally, update the
delete()function to passforcewhen creating the queue:def delete(databusURIs: List[str], databus_key: str, dry_run: bool, force: bool): """ Delete a dataset from the databus. ... """ - queue = DeleteQueue(databus_key) + queue = DeleteQueue(databus_key, force=force)
11-11: Update requests library from 2.28.1 to 2.32.4 or later — multiple critical security vulnerabilities present.requests 2.28.1 is affected by at least three documented CVEs:
- CVE-2023-32681: Proxy-Authorization headers leaked to destination servers during redirects (fixed in 2.31.0)
- CVE-2024-35195: Certificate verification bypassed when Session first request uses verify=False (fixed in 2.32.0)
- CVE-2024-47081: .netrc credentials leaked for certain URLs (fixed in 2.32.4)
Upgrade to 2.32.4 or later (current latest: 2.32.5) to address these vulnerabilities.
🧹 Nitpick comments (2)
README.md (1)
44-48: Documentation note effectively addresses the PyPI release for users.The addition clearly informs users about the version 0.15 release and provides a straightforward upgrade path. This is well-placed and helps resolve the discrepancy mentioned in Issue #35.
Consider slightly refining the wording for improved clarity. The phrase "this repository prepares version" could be more direct—something like "this release is version 0.15" would read more naturally.
🔎 Proposed wording refinement
-Note: the PyPI release was updated and this repository prepares version `0.15`. If you previously installed `databusclient` via `pip` and observe different CLI behavior, upgrade to the latest release: +Note: The PyPI package has been updated to version `0.15`. If you previously installed `databusclient` via `pip` and observe different CLI behavior, upgrade to the latest release:databusclient/api/delete.py (1)
9-9: Consider consistent type hint style.The code mixes legacy (
List[str]fromtyping) and modern (set[str]) type hint styles. For consistency and to align with modern Python practices (PEP 604), consider using lowercase built-in types throughout.🔎 Proposed refactor for consistency
-from typing import List +from typing import Optional - def add_uris(self, databusURIs: List[str]): + def add_uris(self, databusURIs: list[str]): -def _delete_list( - databusURIs: List[str], +def _delete_list( + databusURIs: list[str], -def delete(databusURIs: List[str], databus_key: str, dry_run: bool, force: bool): +def delete(databusURIs: list[str], databus_key: str, dry_run: bool, force: bool):Note: This requires Python 3.9+. If supporting older versions, keep the current approach.
Also applies to: 32-32
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.mddatabusclient/api/delete.py
🔇 Additional comments (3)
databusclient/api/delete.py (3)
1-6: LGTM! Clear module documentation.The module docstring effectively summarizes the deletion utilities and mentions the batching capability via
DeleteQueue.
26-30: LGTM! Clear constructor documentation.
35-38: LGTM! Clear method documentation.The docstrings for
add_uri,add_uris,is_empty, andis_not_emptyare concise and accurately describe their behavior.Also applies to: 42-46, 51-51, 55-55
|
Thanks so far @DhanashreePetare! Back from vacation. I'll take care of the PyPi release. Could you do me a favor and go over the project again to make sure all docstrings are in Google-style docstring format? You've already done a good job, e.g. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
databusclient/api/delete.py (2)
34-38: MissingArgs:section for consistency with Google-style docstrings.The maintainer requested ensuring all docstrings follow Google-style format. This method's docstring is missing the
Args:section documenting thedatabusURIparameter.Suggested fix
def add_uri(self, databusURI: str): """Add a single Databus URI to the deletion queue. + Args: + databusURI: The full Databus URI to add to the queue. + The URI will be deleted when `execute()` is called. """
58-67: Clarify that confirmation already occurred during queueing.The
force=Trueparameter is intentionally set because confirmation happens when items are added to the queue (in_delete_resourcebefore the queue check). Consider updating the docstring to document this design decision explicitly.Suggested docstring enhancement
def execute(self): """Execute all queued deletions. - Each queued URI will be deleted using `_delete_resource`. + Each queued URI will be deleted using `_delete_resource`. Deletions + proceed without further confirmation since items were already confirmed + when they were added to the queue. """
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
databusclient/api/delete.pydatabusclient/api/download.pydatabusclient/api/utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
- databusclient/api/utils.py
🔇 Additional comments (3)
databusclient/api/download.py (1)
36-45: Well-structured Google-style docstrings throughout.The Args sections are properly formatted and the documentation aligns with the maintainer's request for Google-style docstrings. The consistent approach across all functions improves code maintainability.
databusclient/api/delete.py (2)
1-7: Good addition of module-level docstring.Clearly describes the module's purpose and capabilities.
110-121: Docstrings are well-documented with complete Args sections.The documentation clearly explains each parameter's purpose and the function's behavior.
Yeah @Integer-Ctrl, My bad to have the difference in formats, I have updated the docstring format to Google-style now. Please take a look on it. ThankYou. |
2 participants
Description
This PR prepares and enables the publication of databusclient version 0.15 to PyPI, addressing issue #35. The PyPI package is currently outdated (from 2023), making the README installation instructions inconsistent with what users get via
pip install databusclient.Implementation
pyproject.toml(skipping 0.13 and 0.14 as requested)__version__ = "0.15"todatabusclient/__init__.pyfor runtime version checkingRelated Issues
Issue #35
Type of change
Checklist:
poetry run pytest- all tests passedpoetry run ruff check- no linting errorsFor Maintainers: How to Publish to PyPI
Since I don't have PyPI publishing credentials, a maintainer with access needs to complete the publication:
Quick Publish (if you trust the build):
Detailed Instructions
See RELEASE_NOTES.md for comprehensive publishing instructions including TestPyPI testing.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Behavioral Changes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.