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

Fixes and enhancements for the upload API #4417

Merged
merged 6 commits into from Aug 14, 2017

Conversation

Projects
None yet
3 participants
@jmchilton
Copy link
Member

commented Aug 11, 2017

  • 7a1fbd3 - fixes a bug where the content checker simply doesn't work when checking content of compressed uploads.
  • 59aa118 - adds a config option to simple disable upload content checking. If driving Galaxy via CLI for instance - there is no reason to check content.
  • fec1eab - adds an upload option to the API to disable decompressing files. So if someone knows what they are doing - they can stop Galaxy from decompressing big files even if there isn't a decompressed datatype. The uploaded files will simply be of type data,.
  • The remaining commits are tests for all of this as well as the new file:// syntax for uploads and API test enhancements to enable this testing.

jmchilton added some commits Jul 15, 2017

Fix bug in upload HTML checker.
If a chunk was passed into the check_html function - e.g. it was read from a compressed file - it would search through each character in the string for certain HTML content instead of each line in the string.

@jmchilton jmchilton force-pushed the jmchilton:upload_enhance branch from 345219a to 2a706fa Aug 12, 2017

@jmchilton jmchilton force-pushed the jmchilton:upload_enhance branch from 2a706fa to 06ca789 Aug 12, 2017

@jmchilton jmchilton removed the status/WIP label Aug 12, 2017

@@ -14,6 +14,8 @@
def check_html( file_path, chunk=None ):
if chunk is None:
temp = open( file_path, "U" )
elif hasattr(chunk, "splitlines"):
temp = chunk.splitlines()

This comment has been minimized.

Copy link
@bgruening

bgruening Aug 13, 2017

Member

Could we return here an iterator?

This comment has been minimized.

Copy link
@jmchilton

jmchilton Aug 14, 2017

Author Member

We already have the string in memory at this point - so this will only cause another several Kb of memory and it is all native so speed isn't too much of a concern. I was thinking I could rewrite this to use the iterator and control the line read size as well when I realized this piece of code really isn't serving any good purpose - and is causing problems such as preventing Notebooks from being reused. I'd rather revert the fix than spend more time on it at this point 😅.

This comment has been minimized.

Copy link
@bgruening

@bgruening bgruening merged commit f849c7e into galaxyproject:dev Aug 14, 2017

5 checks passed

api test Build finished. 281 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 153 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 44 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Oct 6, 2017

Tests for blocking path pastes and intranet URLs for secondary files...
... during upload.

xref galaxyproject#4289 (blocking intranet URLs by default)
xref galaxyproject#4404 (implementing path pastes for normal uploads with the file:// prefix).
xref galaxyproject#4417 (initial tests for blocking file:// prefixes for non-admins added in galaxyproject#4404)

jmchilton added a commit to jmchilton/galaxy that referenced this pull request Oct 6, 2017

Tests for blocking path pastes and intranet URLs for secondary files...
... during upload.

xref galaxyproject#4289 (blocking intranet URLs by default)
xref galaxyproject#4404 (implementing path pastes for normal uploads with the file:// prefix).
xref galaxyproject#4417 (initial tests for blocking file:// prefixes for non-admins added in galaxyproject#4404)

Rebased with a small fix thanks to the ever vigilant @nsoranzo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.