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

DM-26688: Add command-line tool for Registry.associate #459

Merged
merged 2 commits into from Feb 23, 2021

Conversation

n8pease
Copy link
Contributor

@n8pease n8pease commented Jan 12, 2021

No description provided.

@n8pease n8pease changed the base branch from master to tickets/DM-26689 January 12, 2021 21:11
@n8pease
Copy link
Contributor Author

n8pease commented Jan 12, 2021

This work is built on top of the branch tickets/DM-26689, it will get checked in after that one.

butler = Butler(repo, writeable=True)

if butler.registry.getCollectionType(collection) is not CollectionType.CHAINED:
raise RuntimeError("Can only add datasets to a chained collection.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this check, let's do

butler.registry.registerCollection(collection, CollectionType.TAGGED)
  • This will create the collection if it does not exist, which will be very convenient, and it will raise an exception if it exists with the wrong type.
  • The type needs to be TAGGED, not CHAINED

find_first=find_first,
show_uri=False,
repo=None
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of calling the script rather than the native method to share logic like transforming globs to regexes. Do we need to worry about the script also doing unnecessary things (like making astropy Tables) at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QueryDatasets generates astropy tables on demand via the getter (getTables), so it's structured to not create these if they're not needed. (I do see a mistake in passing the QueryDatasets results to the butler without using the accessor, will fix)

@n8pease n8pease force-pushed the tickets/DM-26689 branch 2 times, most recently from 2fb787d to 18a095a Compare February 2, 2021 18:58
Base automatically changed from tickets/DM-26689 to master February 3, 2021 22:54
@n8pease n8pease force-pushed the tickets/DM-26688 branch 5 times, most recently from 6d683fd to 1419c4d Compare February 4, 2021 21:35
This allows reuse of the options that are needed to call
query datasets.
@n8pease n8pease merged commit d508c2d into master Feb 23, 2021
@n8pease n8pease deleted the tickets/DM-26688 branch February 23, 2021 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants