DM-35391: Improve query-data-ids command #704
Conversation
Codecov Report
@@ Coverage Diff @@
## main #704 +/- ##
==========================================
- Coverage 84.58% 84.57% -0.01%
==========================================
Files 243 243
Lines 31875 31876 +1
Branches 5982 5977 -5
==========================================
- Hits 26961 26959 -2
- Misses 3744 3745 +1
- Partials 1170 1172 +2
Continue to review full report at Codecov.
|
| else: | ||
| return None | ||
| return None, "\n".join(results.explain_no_results()) |
There was a problem hiding this comment.
Is this tuple the convention for returning no results explanations elsewhere in the butler? I feel like raising NoResults (or some other custom exception) with the explanation in the payload would be more pythonic. Looking at the tests, it certainly would simplify this interface.
There was a problem hiding this comment.
This is not a butler convention -- it's trying to bridge the click to script implementation boundary. A specialist exception for this one function seems overkill to me.
|
|
||
| if not graph: | ||
| names = [d.name for d in dataset_types] | ||
| return None, f"No dimensions in common for specified dataset types ({names})" |
There was a problem hiding this comment.
As I comment below on the other return statements, I think this makes more sense raising a custom exception or NoResults.
timj
force-pushed
the
tickets/DM-35391
branch
2 times, most recently
from
fc38b16
to
6ac7068
Compare
Also return the reason for failing to get results so that the command line tool can report extra information.
Checklist
doc/changes