DM-39563: Make clearer error message when repo alias has failed to find something #849
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #849 +/- ##
==========================================
+ Coverage 87.95% 87.96% +0.01%
==========================================
Files 269 269
Lines 35426 35482 +56
Branches 7432 7448 +16
==========================================
+ Hits 31159 31213 +54
- Misses 3123 3125 +2
Partials 1144 1144
☔ View full report in Codecov by Sentry. |
| def clean_environment() -> None: | ||
| """Remove external environment variables that affect the tests.""" | ||
| for k in ( | ||
| "DAF_BUTLER_REPOSITORY_INDEX", | ||
| "S3_ENDPOINT_URL", | ||
| "AWS_ACCESS_KEY_ID", | ||
| "AWS_SECRET_ACCESS_KEY", | ||
| "AWS_SHARED_CREDENTIALS_FILE", | ||
| ): | ||
| os.environ.pop(k, None) |
There was a problem hiding this comment.
Would it make sense to make this more reusable and move to python/lsst/daf/butler/tests/utils.py?
There was a problem hiding this comment.
I'm half inclined to remove the S3 code from here completely but we need one remote datastore for testing and we don't use the webdav one by default. There is lsst.resources.s3utils that tries to have something but it's not quite what we need. I probably should be trying to make that work for the S3 use case at USDF (where pytest doesn't work without this change). The DAF_BUTLER_REPOSITORY_INDEX clean up is really only needed for this one test of testConstructor and a general utility for the S3 tests in butler is only relevant for this one file.
|
I did just push a tweak which catches |
This also includes a more explicit error message rather than relying on the Config() constructor.
This helps to explain why something didn't work.
This makes it easier to work out when no alias resolution has happened.
This allows the logic, especially that revolving around the construction of the error message, to be in one location.
An external DAF_BUTLER_REPOSITORY_INDEX setting can mess up the tests. Additionally S3 env vars have the same effect. The S3 tests also can be corrupted by the environment.
Now works directly with ButlerConfig.
If it's got malformed contents we still want to be able to read a butler without using the alias.
Now cache the actual exception for later retrieval and reset it on each call.
Checklist
doc/changes