feat(download): add EBI-internal FIRE S3 protocol (--protocol fire) - #118
Conversation
Major development to include MassIVE; iPROX and PRoteomeXchange
CPTAC download implementation
Enable to download entire CPTAC dataset
Minor changes for existing folders projects.
FTP/HTTPS/Globus fetches fail for some datasets. Add a `fire` protocol that downloads over the EBI-internal FIRE S3 endpoint (hl.fire.sdo.ebi.ac.uk), which is reachable only inside EBI infrastructure but is far more reliable there. It reuses the existing S3 mechanism (unsigned, bucket pride-public, object key = archive-relative path YYYY/MM/ACCESSION/filename) against the internal endpoint instead of the public hh.fire endpoint used by `s3`. - pride.py: FIRE_S3_URL constant (env-overridable via PRIDEPY_FIRE_ENDPOINT); download_files_from_s3() gains an endpoint_url arg and derives the FTP URL robustly via _get_download_url(); fire routed in _batch_download_by_protocol. - `fire` is requested-only: tried first then falls back to the public protocols, and is never folded into another protocol's fallback (unreachable outside EBI, so no wasted retries there). - CLI: `fire` added to --protocol choices across subcommands + help text. - tests: internal-endpoint selection + S3 key derivation, requested-only order. Equivalent to: aws --no-sign-request --endpoint-url https://hl.fire.sdo.ebi.ac.uk \ s3 cp s3://pride-public/2015/08/PXD002137/<file> .
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough
ChangesFIRE download support
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant CLI
participant PrideProvider
participant FIRE_S3
CLI->>PrideProvider: request fire download
PrideProvider->>PrideProvider: derive archive-relative object key
PrideProvider->>FIRE_S3: request object from FIRE endpoint
FIRE_S3-->>PrideProvider: return file content
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ) | ||
| # boto3.resource was created against the internal hl.fire endpoint. | ||
| assert mock_boto.call_args.kwargs["endpoint_url"] == PrideProvider.FIRE_S3_URL | ||
| assert "hl.fire.sdo.ebi.ac.uk" in PrideProvider.FIRE_S3_URL |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pridepy/tests/test_review_fixes.py`:
- Line 168: Update the assertion for PrideProvider.FIRE_S3_URL in
test_review_fixes.py to avoid hard-coding the default endpoint; validate it
against the configured PRIDEPY_FIRE_ENDPOINT or the expected override-aware
value, while preserving the existing routing assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bab2c31c-f823-48d2-944f-731c19fc38f0
📒 Files selected for processing (3)
pridepy/download/pride.pypridepy/pridepy.pypridepy/tests/test_review_fixes.py
| ) | ||
| # boto3.resource was created against the internal hl.fire endpoint. | ||
| assert mock_boto.call_args.kwargs["endpoint_url"] == PrideProvider.FIRE_S3_URL | ||
| assert "hl.fire.sdo.ebi.ac.uk" in PrideProvider.FIRE_S3_URL |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not hard-code the default endpoint in an override-aware test.
PRIDEPY_FIRE_ENDPOINT can legitimately change FIRE_S3_URL; this assertion then fails despite line 167 confirming that routing used the configured endpoint.
Proposed fix
- assert "hl.fire.sdo.ebi.ac.uk" in PrideProvider.FIRE_S3_URL📝 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.
| assert "hl.fire.sdo.ebi.ac.uk" in PrideProvider.FIRE_S3_URL |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pridepy/tests/test_review_fixes.py` at line 168, Update the assertion for
PrideProvider.FIRE_S3_URL in test_review_fixes.py to avoid hard-coding the
default endpoint; validate it against the configured PRIDEPY_FIRE_ENDPOINT or
the expected override-aware value, while preserving the existing routing
assertion.
Live EBI-cluster test — passed ✅Tested on the EBI Codon login node (inside EBI infrastructure), branch installed into an isolated venv so the shared env was untouched. 1. Endpoint + key derivation — 2. End-to-end CLI — Landed file size This is the same PXD002137 dataset whose FTP/HTTPS fetches were failing; |
A FIRE failure is endpoint-level (the EBI-internal host is unreachable), so it fails identically for every file; boto3 already retries each file 5x within the Phase-1 batch. Retrying fire per-file in Phase-2 just burned retries on connections that cannot succeed. Exclude fire from the per-file fallback so those files go straight to the public protocols. Addresses CodeRabbit review.
Review pass — security + CodeRabbitSecurity review (SSRF / path-traversal / injection focus): clean. The endpoint is set only from a trusted env var / CLI CodeRabbit — 1 actionable finding on this PR, fixed in d78a130:
Fixed: (CodeRabbit's other two findings were on Tests: 13 passed in |
2 participants
Summary
Some datasets fail to download over FTP / HTTPS / Globus. This adds a
fireprotocol that fetches over the EBI-internal FIRE S3 endpoint (hl.fire.sdo.ebi.ac.uk) — reachable only inside EBI infrastructure (compute/login nodes), where it is markedly faster and more reliable than the public paths.It reuses the existing S3 mechanism already used by the
s3protocol (unsigned/anonymous, bucketpride-public, object key = the archive-relative pathYYYY/MM/ACCESSION/filename), differing only in the endpoint host:s3→ publichh.fire,fire→ internalhl.fire.Equivalent to the manual command:
Changes
download/pride.pyFIRE_S3_URLconstant (hl.fire.sdo.ebi.ac.uk), overridable via thePRIDEPY_FIRE_ENDPOINTenv var.download_files_from_s3()gains anendpoint_urlparameter and now derives the FTP URL robustly via_get_download_url(record, "ftp")instead of positionalpublicFileLocationsindexing.firerouted in_batch_download_by_protocol(internal endpoint) alongsides3(public endpoint); added to_get_download_urland the supported-protocol list._protocol_sequence("fire") == ["fire", "aspera", "s3", "ftp", "globus"], andfireis never folded into another protocol's fallback chain — it is unreachable outside EBI, so it would only waste retries there. A run started outside EBI still completes via the public protocols.pridepy.py—fireadded toPROTOCOL_CHOICES(so every subcommand, incl.download-files-by-list, accepts it) and help text.2015/08/PXD002137/a.raw), and the requested-only fallback ordering.Usage
Validation
pytest pridepy/tests/— 152 passed, 4 skipped (network).--protocol fireagainst a real dataset to follow in a comment.Notes
fireendpoint is internal to EBI; outside EBI it will fail fast and fall back to the public protocols, so the flag is safe to pass anywhere.Summary by CodeRabbit
New Features
fireas a supported download protocol.Documentation