Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor uploads via CLI #179

Merged
merged 8 commits into from
Jan 23, 2019
Merged

Refactor uploads via CLI #179

merged 8 commits into from
Jan 23, 2019

Conversation

polyatail
Copy link
Contributor

This PR addresses #105, #108, and #109 by:

  • Removing FASTX validation code and deleting inline_validator.py
  • No longer distinguishing between small and large uploads--all go through same code path
  • Continue to interleave FASTX files when paired via FASTXInterleave object
  • FASTXInterleave decompresses when necessary, interleaves, and uploads uncompressed
  • Upload single-ended files via FASTXPassthru which leaves files as-is
  • Switch filter_reads.py to skbio.io FASTX parser, having removed our own parsers

In addition, these changes were made:

  • Progress bar during upload now uses click.progressbar
  • Errors in multithreaded upload are printed as they happen rather than at the end
  • Passing -v sets loglevel to DEBUG rather than INFO
  • pytest upgraded to 3.3.0
  • Tests using old make_creds_file() were switched over to mocked_creds_file fixture

@polyatail polyatail added the done ready to be merged label Jan 17, 2019
@polyatail polyatail added this to the Sprint Kiwi milestone Jan 17, 2019
@polyatail polyatail self-assigned this Jan 17, 2019
@boydgreenfield
Copy link
Contributor

boydgreenfield commented Jan 18, 2019

I think this is our checklist of TODOs before removing the validation code:

  • Add a retry_url or similar param in cases where our proxy fails with a 500 (which will not have quite as good uptime as S3, hard as we try)
  • Add a hook to call /samples/<id>/cancel_upload when we cancel a pending upload
  • Fix any remaining proxy edge cases that may be blocking (see our internal proxy repo)

@polyatail polyatail added in progress not yet ready for review and removed done ready to be merged labels Jan 18, 2019
@coveralls
Copy link

coveralls commented Jan 19, 2019

Coverage Status

Coverage decreased (-0.3%) to 83.858% when pulling a1495a9 on roo-upload into 7edf4b2 on master.

* Removed file validation code. Closes #105.
* Do not distinguish between small and large uploads. Closes #108.
* Keep uploading threading code for now. Closes #109.
* Use skbio parsers in filter_reads since ours were removed.
* Upgraded pytest to 4.1.1.
@polyatail polyatail force-pushed the roo-upload branch 2 times, most recently from 5842f3d to c77a5f8 Compare January 21, 2019 20:39
@polyatail polyatail added done ready to be merged and removed in progress not yet ready for review labels Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
done ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants