refactor: collapse Files shim layer (0.0.18)#104
Conversation
Migrated ~17 patch.object(Files, "X") test targets to canonical locations:
- PRIDE methods now patched on PrideProvider
- transport/util helpers patched on transport / util modules
- by_url helpers patched on commands.by_url
Moved PRIDE protocol workers permanently into PrideProvider:
- download_files_from_{aspera,globus,s3,ftp}
- _batch_download_by_protocol, _download_with_fallback, _globus_download_one
- _protocol_sequence, download_private_file_name, stream_all_files_by_project
- get_submitted_file_path_prefix, save_checksum_file, get_ascp_binary,
get_output_file_name
Eliminated every from pridepy.files.files import Files lazy import inside
providers/ and commands/. Providers call self.X / transport.X / util.X
directly. Zero back-into-Files coupling.
Files dropped from 912 to 382 LOC: only public CLI methods + class-attribute
constant re-exports. Kept ~5 one-line @staticmethod shims for likely
downstream imports (compute_md5, validate_download, read_checksum_file,
download_ftp_urls, download_http_urls) plus the accession-matcher helpers
(is_massive_accession etc. and is_direct_download_accession) as useful API.
No behaviour change. No test assertion changes. 68 passed, 4 skipped.
Flake8 (--select=E9,F63,F7,F82) clean.
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
Superseded by #105, which now targets master and contains this branch's commits as part of the consolidated 0.0.16 release. Closing to keep a single PR. (Branch left intact.) |
1 participant
Summary
Follow-up to #103. Collapses the backward-compat shim layer in
pridepy/files/files.pyso providers own their own logic andFilesis a true thin facade.files.py: 912 → 382 LOC. No behaviour change. No test assertion changes. Bumps to 0.0.18.What changed
Test patches migrated to canonical locations (17 sites across 5 test files):
PrideProvider(_batch_download_by_protocol,_download_with_fallback,_globus_download_one,stream_all_files_by_project)pridepy.providers.transport(download_ftp_urls,download_http_urls)pridepy.providers.util(validate_download)pridepy.commands.by_url(_http_download_url,_download_single_url)patch.objecttargets changed; assertions untouched.PRIDE protocol workers moved permanently into
PrideProvider:download_files_from_aspera,get_output_file_name(the rest were already there). No shim left onFiles.Every
from pridepy.files.files import Fileslazy import removed fromproviders/andcommands/. Providers now callself.X/transport.X/util.Xdirectly.util._get_download_urlreads URL-prefix constants fromPrideProviderinstead ofFiles. Zero back-into-Files coupling (verified:grepreturns 0).~60 shim methods deleted from
Files. Kept as one-line@staticmethodshims for likely downstream imports:compute_md5,validate_download,read_checksum_file,download_ftp_urls,download_http_urls, plus the accession matchers (is_massive_accessionetc.,is_direct_download_accession,_repo_uses_tls). Class-attribute constant re-exports preserved.What
FileskeepsPublic CLI methods only (each a thin registry dispatch):
download_all_raw_files,download_all_category_files,get_all_raw_file_list,get_all_category_file_list,download_file_by_name,get_file_from_api,download_files_by_url,download_files_by_list,download_px_raw_files,stream_all_files_metadata,get_submitted_file_path_prefix— plus the constant re-exports and compat shims above.Verification
--select=E9,F63,F7,F82): 0 errorspridepy --version: 0.0.18download-file-by-name -a MSV000080175 -f params.xml→ MD543d87368d705c3f380c1d030b14850c4grep -rn "from pridepy.files.files import Files" pridepy/providers/ pridepy/commands/→ 0 resultsgrep -rn "patch.object(Files," pridepy/tests/→ 0 resultsBase-branch note
Targets
feat/files-py-modular-architecture(#103) so the diff is small. After #103 merges to dev, update this PR's base.A follow-up (#105) will restructure the
Providerbase into a Template-Method shape (rich base class owns the download workflow; adapters fill in provider-specific holes).Test plan
wc -l pridepy/files/files.pyreports < 40043d87368d705c3f380c1d030b14850c4pridepy --versionreports0.0.18