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-24347: Allow a component dataset to be None #262

Merged
merged 5 commits into from Apr 24, 2020
Merged

Conversation

timj
Copy link
Member

@timj timj commented Apr 23, 2020

Sometimes when we store a dataset, not all the components are defined. Registry won't register them in that case but there are some code paths that will still cause a datastore to try to return a None component.

@TallJimbo
Copy link
Member

Code looks good; my only quibble is with the message on the final commit: Registry does contain components that are None, probably more often than not - we just register all components in the StorageClass every time we put or ingest, regardless of whether they're present on the in-memory dataset or file.

I don't think that invalidates anything you've done here, but I think the message is wrong (or maybe just confusing, if I misunderstood it).

@timj
Copy link
Member Author

timj commented Apr 23, 2020

Yes, you are right when you don't disassemble the in-memory dataset. If you disassemble then we do not register the None components. This means that when I added some tests for None to test_butler.py they were all blowing up in the cases where we do disassemble.

InMemoryDatastore can not ingest so in a ChainedDatastore
it will complain when a dataset is trashed that it knew
nothing about. We may want to have finer control of logging
when a ChainedDatastore is involved since chaining by definition
allows datasets to be added to some child datastores but
not others.
Sometimes when we store a dataset, not all the components
are define. This happens when datasets are disassembled.
In other cases we do get a registry entry for each
component and that component might be None.
Be explicit about whether we expect a None or a real entity.
@timj timj merged commit 979aa60 into master Apr 24, 2020
@timj timj deleted the tickets/DM-24347 branch April 24, 2020 02:26
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