DM-43431: In SasquatchDatastore allow namespace and extra fields to come from environment #228
Conversation
python/lsst/analysis/tools/interfaces/datastore/_sasquatchDatastore.py
Outdated
Show resolved
Hide resolved
timj
force-pushed
the
tickets/DM-43431
branch
2 times, most recently
from
e26fb93
to
f3f4772
Compare
There was a problem hiding this comment.
Mostly just documentation changes.
However, I do have one extra request: can you update the IDENTIFIER_KEYS constant in _dispatcher.py to include group and day_obs? It seems to have been overlooked when they were implemented.
python/lsst/analysis/tools/interfaces/datastore/_sasquatchDatastore.py
Outdated
Show resolved
Hide resolved
| @@ -80,6 +81,9 @@ class SasquatchDatastore(GenericBaseDatastore): | |||
| namespace: str | |||
| """The namespace in Sasquatch where the uploaded metrics will be | |||
| dispatched. | |||
|
|
|||
| Defaults to the value obtained from the ``$DAF_BUTLER_SASQUATCH_NAMESPACE`` | |||
| environment variable and "lsst.dm" if no environment variable is defined. | |||
There was a problem hiding this comment.
The wording here is a bit confusing, assuming I understand correctly that these fields can all be set directly in config. Maybe:
| environment variable and "lsst.dm" if no environment variable is defined. | |
| Overridden by the ``$DAF_BUTLER_SASQUATCH_NAMESPACE`` environment variable | |
| if it is defined. Defaults to "lsst.dm". |
There was a problem hiding this comment.
Should this have an explicit default like defaultConfigFile does?
There was a problem hiding this comment.
Sorry. I don't know what you mean. The default is lsst.dm.
There was a problem hiding this comment.
__init__ assigns "lsst.dm" if nothing supersedes that. I meant "should something be assigned to namespace here, where it's declared?"
There was a problem hiding this comment.
Ah. Okay. We could do that. Generally the class properties are there for docstrings and I'm not sure why Nate didn't set the default there instead, although a class property and an instance property can get confusing.
python/lsst/analysis/tools/interfaces/datastore/_sasquatchDatastore.py
Outdated
Show resolved
Hide resolved
python/lsst/analysis/tools/interfaces/datastore/_sasquatchDatastore.py
Outdated
Show resolved
Hide resolved
timj
force-pushed
the
tickets/DM-43431
branch
2 times, most recently
from
9eaf31e
to
a4b9197
Compare
|
@kfindeisen please take another look. Completely rewritten. Thanks for the comments. |
Namespace from environ overrides that from config.
The fields can come from configuration and the environment, with the environment taking priority.
These are new dimensions that will appear in repos soon.
No description provided.