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-28315: Create Registry ABC #503

Merged
merged 14 commits into from Apr 7, 2021
Merged

DM-28315: Create Registry ABC #503

merged 14 commits into from Apr 7, 2021

Conversation

timj
Copy link
Member

@timj timj commented Apr 6, 2021

No description provided.

@timj timj force-pushed the tickets/DM-28315 branch 2 times, most recently from 3f039d4 to d457959 Compare April 6, 2021 21:40
timj added 5 commits April 6, 2021 14:56
The code imported items that were not explicitly exported and
subject to failures when code is reorganized.
* Rename old Registry -> SqlRegistry
* Registry.fromConfig now loads class name from config
These need some thought but add them for now to placate mypy.

Datastore client/server is going to be complicated.
@timj
Copy link
Member Author

timj commented Apr 6, 2021

I've redone the commit history to try to give git a chance to handle the changes in #500 when either that is rebased onto this or this is rebased onto that.

Copy link
Contributor

@andy-slac andy-slac left a comment

Choose a reason for hiding this comment

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

Looks good, small bunch of minor comments.

python/lsst/daf/butler/registry/_registry.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/registry/_registry.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/registry/_registry.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/registry/_registry.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/registry/_registry.py Show resolved Hide resolved
python/lsst/daf/butler/registry/_registry.py Show resolved Hide resolved
python/lsst/daf/butler/registry/_sqlRegistry.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/registry/_sqlRegistry.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/registry/tests/_registry.py Outdated Show resolved Hide resolved
tests/test_cliCmdPruneDatasets.py Show resolved Hide resolved
@timj
Copy link
Member Author

timj commented Apr 7, 2021

@andy-slac Thanks. Some of your comments are from existing code that I moved around but I will take a look at them. I was minimizing changes to the old Registry class since I know we have code changes for that class in flight and I was worried about the rebase conflict.

@timj timj merged commit 3ff10e8 into master Apr 7, 2021
@timj timj deleted the tickets/DM-28315 branch April 7, 2021 19:39
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