Fix duplicate GET request during Vault-authenticated downloads#53
Fix duplicate GET request during Vault-authenticated downloads#53Dev10-sys wants to merge 4 commits intodbpedia:mainfrom
Conversation
📝 WalkthroughWalkthroughRemoved the explicit Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (user)
participant Client as DatabusClient
participant Vault as Vault
participant Server as Storage Server
participant FS as Local FS
CLI->>Client: download --validate-checksum <object>
Client->>Vault: request auth token
Vault-->>Client: auth token
Client->>Server: GET /object (no forced Accept-Encoding)
Server-->>Client: response (maybe gzipped) + checksum metadata
Client->>FS: stream write file
Client->>Client: compute checksum
alt checksum matches
Client->>CLI: success
else checksum mismatch
Client->>CLI: error / fail
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 200-201: The README flag description for --validate-checksum says
it will "fail on mismatch" but the CLI help text in cli.py (the add_argument
call for "--validate-checksum") only says "Validate checksums of downloaded
files"; update the help string in the add_argument/argparse setup (the
"--validate-checksum" argument) to match README by appending the failure
behavior (e.g., "Validate checksums of downloaded files and fail on mismatch")
so both docs are consistent.
- Around line 297-299: The README option list is out of sync with the CLI: the
options --title, --abstract, and --description are declared with required=True
in cli.py, so update the README entries for these three flags to include the
“[required]” marker to match the actual CLI help output (alternatively, if you
intend them to be optional, remove required=True from the corresponding option
definitions in cli.py for --title, --abstract, and --description so the docs
remain correct).
|
Thanks for the review. Updated the README to align with the actual CLI output, including marking --title, --abstract, and --description as required. Also aligned the --validate-checksum description between README and CLI help text to clearly indicate failure on checksum mismatch. |
| /openid-connect/token] | ||
| --clientid TEXT Client ID for token exchange [default: vault- | ||
| token-exchange] | ||
| --convert-to [bz2|gz|xz] Target compression format for on-the-fly | ||
| conversion during download (supported: bz2, gz, | ||
| xz) | ||
| --convert-from [bz2|gz|xz] Source compression format to convert from | ||
| (optional filter). Only files with this | ||
| compression will be converted. |
There was a problem hiding this comment.
Your changes delete existing CLI options! Do the descriptions of the CLI options need updating on your branch?
What this PR does
Accept-Encodingheader during Vault-authenticated downloadsWhy
Scope
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.