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

Added support for recursively discovering output datasets #5240

Merged
merged 3 commits into from Jan 17, 2018

Conversation

Projects
None yet
5 participants
@nuwang
Member

nuwang commented Dec 21, 2017

This PR adds a recurse flag for discovering output datasets, so that directory trees containing output can be discovered.

Usage:

<discover_datasets pattern="__name__" directory="subdir1" ext="txt" recurse="true" />

ping @jmchilton

@galaxybot galaxybot added the triage label Dec 21, 2017

@galaxybot galaxybot added this to the 18.01 milestone Dec 21, 2017

if extra_file_collector.directory:
directory = os.path.join(directory, extra_file_collector.directory)
def discover_target_directory(dir_name, job_working_directory):
if dir_name:

This comment has been minimized.

@mvdbeek

mvdbeek Dec 31, 2017

Member

This isn't equivalent to the previous version, and I guess this is why collecting outputs from tool provided metadata fails (see https://jenkins.galaxyproject.org/job/docker-framework/9815/).
In line 477 this function is called. I suppose changing the call in 477 to pass in extra_file_collector.directory instead of extra_file_collector may fix the failing tests.

This comment has been minimized.

@nuwang

nuwang Jan 13, 2018

Member

@mvdbeek Thanks for catching that - I'd missed your comment and I've just pushed in the fix as suggested. I changed it around a few times and must have forgotten to update the parameter.

This comment has been minimized.

@martenson

martenson Mar 2, 2018

Member

it seems to me this method may still have problems xref: #5560

@jmchilton jmchilton modified the milestones: 18.01, 18.05 Jan 2, 2018

@nuwang

This comment has been minimized.

Member

nuwang commented Jan 15, 2018

@mvdbeek Is there a way to retrigger the build? The other PR passes fine after the latest changes, and it includes all then changes in this PR, so it looks like the latest commit got caught to a temporary Travis issue.

@mvdbeek mvdbeek closed this Jan 15, 2018

@mvdbeek mvdbeek reopened this Jan 15, 2018

@mvdbeek

This comment has been minimized.

Member

mvdbeek commented Jan 15, 2018

Closing and opening again will trigger the tests (at least on travis).

@nuwang

This comment has been minimized.

Member

nuwang commented Jan 15, 2018

@mvdbeek Good to know, thanks. It's passing now.

@mvdbeek

Thanks @nuwang, that's definitely a useful feature!

@nuwang

This comment has been minimized.

Member

nuwang commented Jan 16, 2018

@mvdbeek Thanks very much for reviewing!
@jmchilton Is it too late to merge this into the 18.01 release? Ted was pretty keen to get the multi-file upload going so it would be great if we can still merge this in if we are not past the window.

@martenson martenson merged commit b91d8e1 into galaxyproject:dev Jan 17, 2018

6 checks passed

api test Build finished. 343 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 168 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 67 tests run, 0 skipped, 0 failed.
Details
selenium test Build finished. 118 tests run, 2 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@martenson

This comment has been minimized.

Member

martenson commented Jan 17, 2018

@nuwang this will make it to the 18.01, thank you for the contribution!

@nuwang nuwang deleted the gvlproject:discover_datasets_recursive branch Jan 18, 2018

@martenson martenson modified the milestones: 18.05, 18.01 Jan 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment