DM-37499: Improve log message for datastore.mexists #848
Conversation
| if n_results == 1: | ||
| ref = list(chunk_result)[0] | ||
| log.log( | ||
| log_threshold, |
There was a problem hiding this comment.
It's possible this should be a DEBUG message since we don't log anything for the simple datastore.exists() API. The multi-chunk case if VERBOSE because it takes a significant amount of time to run and it's good to have feedback. The middle ground is the single chunk and whether the verbose vs debug depends on the amount of time that it took to do the existence check (9999 datasets being checked will obviously take much longer than 2 datasets).
There was a problem hiding this comment.
I don't have strong opinion, but I'm a big fan of trying to keep things simple. 🙂 Logging everything at VERBOSE may be OK, as I think most people will run it with INFO.
There was a problem hiding this comment.
We always run with VERBOSE during workflow execution (mainly because we need to issue log messages when things take a while in order to let people know that the task is still running).
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #848 +/- ##
==========================================
- Coverage 87.93% 87.93% -0.01%
==========================================
Files 268 268
Lines 35254 35262 +8
Branches 7393 7395 +2
==========================================
+ Hits 31002 31008 +6
- Misses 3112 3113 +1
- Partials 1140 1141 +1
☔ View full report in Codecov by Sentry. |
|
I didn't have an easy way to test the chunking (and no programmatic way to shrink the chunk size) so that code path doesn't trigger in the tests. |
* Use different log message for single file vs single chunk vs multiple chunks.
Checklist
doc/changes