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-18577: Support butler relocation #141

Merged
merged 16 commits into from Mar 27, 2019
Merged

DM-18577: Support butler relocation #141

merged 16 commits into from Mar 27, 2019

Conversation

timj
Copy link
Member

@timj timj commented Mar 25, 2019

  • Allow a datastore/registry root to be specified relative to the location of the butler.yaml file.
  • Allow the name of a posix datastore to be specified in the config file.

@timj timj requested a review from TallJimbo March 25, 2019 21:27
Copy link
Member

@TallJimbo TallJimbo left a comment

Choose a reason for hiding this comment

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

A few minor (mostly clarification) comments. My only big-picture concern (which is also a minor one) is that - if I've understood correctly - sometimes the root in a Config instance is expanded, and sometimes it isn't, and that means knowing what to expect in one depends on some state that's hard to track. If it's not too hard, I'd prefer for there to be a different attribute (maybe in the Config, maybe in the things that use one) that's always expanded so the one in the Config can always be unexpanded. But if there's no good place to put that attribute, I could certainly believe that suggestion isn't worth the cost.

Mind updating the commit messages for the two YAML commits to make it clear that the unsafe loading is in gen2convert? Right now, looking at just the messages, one would get the impression that one commit reverts the other.

config/datastores/posixDatastore.yaml Outdated Show resolved Hide resolved
python/lsst/daf/butler/butler.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/core/butlerConfig.py Show resolved Hide resolved
python/lsst/daf/butler/core/repoRelocation.py Outdated Show resolved Hide resolved
python/lsst/daf/butler/core/repoRelocation.py Outdated Show resolved Hide resolved
@timj
Copy link
Member Author

timj commented Mar 26, 2019

@TallJimbo I don't understand your main comment about being uncertain about whether something will be expanded or not. If you don't use <root> in your config file then there is no change from the current behavior. If you use <root> then in all standard cases it will be replaced either with the value of root at the top of the butler config, or the directory containing the butler.yaml file you have asked to read. The only time you may be unclear as to what is going on is if your butler is created with a ButlerConfig that was created without the use of a YAML file. In that case current working directory will be used for the root (and I could issue a warning for that rare case).

Copy link
Member

@TallJimbo TallJimbo 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!

timj added 15 commits March 27, 2019 12:55
We have no control over the formatter yaml output and to read
gen2 repository configuration YAML we also need to use an
unsafe loader.
Use "<root>" in a configuration file to specify that
you are relative to the butler root directory.
Some of the tests were loading the butler.yaml file directly
which triggered POSIXDatastore instance to create a new directory
in tests/configs/basic (where the butler.yaml file was located).
Previously these had gone into the daf_butler root directory.

Now modify the tests to always use a temp directory.
This allows the butler configuration to be in one place but the
datastore and registry to be in a different directory tree.
…erConfig

Seeding one from another means they do have the same original
configuration directory.
This ensures that the butler resources are freed and without
causing difficulties with registry disconnection.
with Butler() as butler:
    # Do something
Whilst it seemed like a good idea to put all the datastore files
in a standalone subdirectory, this does confuse ci_hsc a bit
so for now leave it out. Can revisit in the future.
Decide against using '{root}' because the curly brace is
special syntax in YAML and would require that the root be
quoted every time.

This change cleans up a test so that the root tag is
retrieved from code and cleans up the documentation
so that the root tag is documented in sphinx without
hard coding the value of the special tag into a
docstring.
Use `noqa: Nxxx` syntax to hide specific codes.
@timj timj merged commit 778f0db into master Mar 27, 2019
@timj timj deleted the tickets/DM-18577 branch March 27, 2019 22:50
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