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

Re-work upload clarification from #5206. #5264

Merged
merged 1 commit into from Jan 4, 2018

Conversation

jmchilton
Copy link
Member

Re-work upload clarification from #5206.

See post-merge discussion on that issue.

See post-merge discussion on that issue.
@galaxybot galaxybot added this to the 18.01 milestone Jan 4, 2018
@mvdbeek
Copy link
Member

mvdbeek commented Jan 4, 2018

👍 I think that's correct now.

@martenson martenson merged commit be5b88e into galaxyproject:dev Jan 4, 2018
@nsoranzo
Copy link
Member

nsoranzo commented Jan 4, 2018

This looks good to me, but do we have any test for the purge_source upload option? It seems that should have failed before.

@jmchilton
Copy link
Member Author

@nsoranzo Well... in fact... https://github.com/galaxyproject/galaxy/blob/dev/test/integration/test_upload_configuration_options.py#L264.

        # Purge is set by default so this should be gone.
        # ... but it isn't - is this a bug? Are only certain kinds of uploads purged?
        # assert not os.path.exists(ftp_path)

@jmchilton
Copy link
Member Author

Well I played a bit more with that test case and it seems like purge would only ever be used with non-binary data if the to posix lines option is disabled - because usually Galaxy will create a new file and not purge the original. I may open a PR to clarify some of this in testing. I guess an open question is if we should purge those files that we don't move - just to keep things clean and consistent across datatypes and data content.

@nsoranzo
Copy link
Member

nsoranzo commented Feb 21, 2018

@jmchilton @mvdbeek I just find out (after a long session of git bisect) that this is the commit causing the BioBlend test failures at https://travis-ci.org/galaxyproject/bioblend/builds/339232363 for 18.01 and dev. When testing upload to libraries from filesystem paths, the files to upload are moved (instead of copied) at https://github.com/galaxyproject/bioblend/blob/master/bioblend/_tests/TestGalaxyLibraries.py#L61 , so the upload of the same files in the next line fails.

I think we should modify tools/data_source/upload.py as follows:

purge_source = dataset.get('purge_source', True) and not run_as_real_user and dataset.type not in ('server_dir', 'path_paste')

nsoranzo added a commit to nsoranzo/galaxy that referenced this pull request Feb 21, 2018
nsoranzo added a commit to nsoranzo/galaxy that referenced this pull request Feb 21, 2018
@nsoranzo nsoranzo deleted the upload_clarification_2ish branch August 31, 2022 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants