PDC integration: one downloader for both data commons - #4
Closed
paulocilasjr wants to merge 5 commits into
Closed
Conversation
Member
|
functionality added via #5 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the GDC-only downloader with a tool that takes a manifest from either the
Genomic or Proteomic Data Commons, detects which produced it, and returns a
collection that is usable by downstream tools without manual conversion.
tools/gdc-downloader/→tools/gacdi-downloader/; tool idgdc_downloader→gacdi_downloader.What it does
Detection (
download/detect.py) reads the manifest header and picks GDC vsPDC from column names. Matching neither, or both, fails with the observed and
expected columns rather than guessing.
PDC transfers (
download/pdc.py) stream to a.partialpath, verify size andMD5 before moving into place, and skip rows already present with the right
checksum — PDC allows only 10 downloads of a file per 24h per IP. Expired signed
URLs (7-day lifetime) and rate limits get actionable messages. Duplicate names are
disambiguated by file ID; path separators and
..are rejected before any write.GDC transfers keep the existing
gdc-clientwrapper and its mode-0600 namedpipe for controlled-access tokens.
Gzipped payloads are expanded after checksum verification, so integrity is
checked against the bytes the manifest describes. This is what makes the output
usable — Galaxy's
mzml/mziddatatypes describe uncompressed XML, so.mzML.gzcan't reach msconvert, Comet or OpenMS without a conversion step. Expansion uses
an allow-list of inner extensions, not magic bytes: BAM, BGZF VCF and tabix
indexes are all gzip but meaningless expanded.
--keep-compressedexists at theCLI, not as a tool parameter.
Discovery, and two silent bugs
Unregistered extensions gave a collection that opened empty.
__name_and_ext__hands the trailing filename token to the datatype registry, but commons files are
named
.mzML,.mzid,.psm,.rawwhere Galaxy wantsmzml,mzid,tabular,thermo.raw. Galaxy setelement_countwhile the elements stayedunresolvable: "a list with 5 datasets" that opened empty.
Datatypes are now declared explicitly, one rule per format, with a catch-all
keeping unknowns as
datainstead of dropping them. Sniffing was rejected — thesefiles reach hundreds of MB and several are binary. Patterns are anchored and the
catch-all uses a negative lookahead, so every file matches exactly one rule.
Sorting on an uncaptured group errored the collection. The rules sorted on
name, which no pattern captures. Galaxy sorts withattrgetter(sort_by)andJsonCollectedDatasetMatch.namereturnsNone, so sorting two matches raisedTypeError. The tool had already exited 0, so the only symptom was an erroredcollection with clean stderr — and it needed two files matching one rule to fire,
so a manifest with one file per datatype passed. Now sorts on
designation.Tests
Transfer logic is covered offline: checksum and size mismatches, partial cleanup,
retry budget, expired URLs, rate limits, path traversal, duplicate names, resume,
and that integrity is checked against the compressed bytes.
Two contract tests need a reviewer's judgement, since they encode Galaxy behaviour
in our suite.
test_wrapper_discovery_contract.pyreplicates whatoutput_collectrequires of these patterns; reintroducing thesort_bybug failsit in 14 places.
test_version_contract.pyties each wrapper's@TOOL_VERSION@tothe package
__version__. Trade-off: if Galaxy changes how it sorts, these modelthe old behaviour and could pass while the wrapper breaks. Both bugs above were
invisible to the tool test and to
planemo lint, which is why I took that.The GDC tool test hits the live API, so it only runs under
planemo test. PDC hasno live test on purpose: signed URLs expire in 7 days and each run burns quota.
Versioning
__version__→0.3.0, both wrappers with it.containers.ymltags both imagesfrom that version and refuses to overwrite, so a suffix-only bump would have failed
the build —
gacdi-downloader:0.2.0is already published.manifest_gdcwaspinned at
0.1.0against a0.2.0package, orphaning a tag on every build;test_version_contract.pyprevents that recurring.Expect one red CI run after merge
Both jobs now verify the image is publicly pullable, querying Quay without
credentials — the workflow is logged in, so an authenticated check would pass on a
private repo and prove nothing.
goeckslab/gacdi-downloaderdoesn't exist on Quay yet, and Quay createsrepositories private, so this step fails the first time it runs. That's the
check working: otherwise the push succeeds and every Galaxy job dies at pull time
with an auth error that says nothing about visibility. Fix once after merge: let
it push, set the repo public in Quay settings, re-run.
Review checklist
gacdi-downloaderpublic on Quay after the first push.txt.gz/.maf.gz, protects.bam/.vcf.gz/.tbi)gdc_downloader