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

Library tags #4262

Merged
merged 17 commits into from Aug 9, 2017

Conversation

@bwlang
Copy link
Contributor

commented Jun 28, 2017

This adds support for #tags in library datasets, auto-tagging on input (based on filenames) and copying tags to history datasets on import.

@dannon

This comment has been minimized.

Copy link
Member

commented Jun 28, 2017

Nice stuff! I'll check this out today

@dannon dannon self-requested a review Jun 28, 2017

@bwlang

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2017

great! - thanks @dannon. I just committed a fix for a style violation, can you trigger the other tests (I can't easily run them locally due to tox incompatibility with conda)

@galaxybot galaxybot added the triage label Jun 28, 2017

@galaxybot galaxybot added this to the 17.09 milestone Jun 28, 2017

@bwlang

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2017

I don't think you care about what I tell you ... but I'll try anyway
@galaxybot test this

@nsoranzo

This comment has been minimized.

Copy link
Member

commented Jun 28, 2017

@bwlang galaxybot only answers to people in the committers group.

@jmchilton

This comment has been minimized.

Copy link
Member

commented Jul 4, 2017

I'll work on eliminating the transient failures. It looks like some of these are legitimate failures. @bwlang do you think you are going to have time to follow up on this? If you'd like a committer to follow up and hack on this some let us know - I can't promise anything but this is a very nice start and we should try to get this in for 17.09.

@bwlang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2017

@jmchilton : I have some time now... i'll at least try to suss out which tests are telling me something and which are not.

@bwlang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2017

@jmchilton I think this will fix at least a few of these tests...

@bwlang bwlang force-pushed the bwlang:library_tags branch from e63dda0 to aee6337 Jul 4, 2017

@bwlang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2017

forced a rebase to sync up with upstream/dev... hopefully will run tests now.

@galaxyproject galaxyproject deleted a comment from dannon Jul 4, 2017

@galaxyproject galaxyproject deleted a comment from dannon Jul 4, 2017

@bwlang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2017

@galaxybot test this ( I think this test only works sometimes)

@bwlang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2017

@dannon I think the remaining failure is actually one of the tests that is not deterministic and this is ready for review.

for the failing test... Maybe it's operating on a partial download of the tar archive?

@bwlang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2017

@galaxybot test this

@martenson martenson removed the request for review from dannon Jul 5, 2017

@dannon dannon self-requested a review Jul 5, 2017

@dannon

This comment has been minimized.

Copy link
Member

commented Jul 5, 2017

@bwlang Yep, I think you're right. I'll go through this now, thanks for the updates!

@bwlang bwlang force-pushed the bwlang:library_tags branch from 393fbd0 to 73012c9 Jul 6, 2017

@bwlang

This comment has been minimized.

Copy link
Contributor Author

commented Jul 6, 2017

@dannon , just rebased so this can apply to dev

@dannon

This comment has been minimized.

Copy link
Member

commented Jul 6, 2017

Had a couple of problems with this. sa_session wasn't defined in my testing, and I'd also like to retain tags added to data libraries from histories. I'll try to open a PR against your branch later today. (PR merged now, the rest of the work can be future improvements)

@martenson martenson added this to Feature in Data Libraries Aug 1, 2017

Merge pull request #1 from dannon/library_tags
Swap two more spots initializing TagManagers to new session
@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 8, 2017

@martenson what do you think?

@martenson
Copy link
Member

left a comment

This is a fairly narrow usecase but a solid groundwork for future improvements. Thank you Brad!

martenson added some commits Aug 9, 2017



LibraryDatasetDatasetAssociationTagAssociation_table = Table( "library_dataset_dataset_association_tag_association", metadata,
Column("id", Integer, primary_key=True),

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Aug 9, 2017

Member

Indentation doesn't seem correct here.

@martenson martenson merged commit 4297718 into galaxyproject:dev Aug 9, 2017

0 of 5 checks passed

api test Test scheduled.
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
framework test Test scheduled.
Details
integration test Test scheduled.
Details
toolshed test Test scheduled.
Details

@martenson martenson moved this from Feature to Done in Data Libraries Aug 9, 2017

@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 13, 2017

Double check of the migration can be found in this commit: bgruening/docker-galaxy-stable@1eea028

@martenson martenson moved this from Done to Closed in Data Libraries Aug 30, 2017

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.