RF: Add logging, dissolve duct_print (INFO level), add CLI option -l, dissolve --quiet #140
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #140 +/- ##
==========================================
- Coverage 95.26% 94.59% -0.67%
==========================================
Files 2 2
Lines 401 407 +6
Branches 63 62 -1
==========================================
+ Hits 382 385 +3
- Misses 12 14 +2
- Partials 7 8 +1 ☔ View full report in Codecov by Sentry. |
… dissolve --quiet When we have logging, and it also goes to stderr, there is really no need for some dedicated "duct_print". Now everything can be controlled with -l and no additional --quiet is needed. the only location is when to report that command is incorrect - that is where I used explicit print to stderr since so far the only location. I also added a few lgr.debug level statements.
yarikoptic
force-pushed
the
enh-logging
branch
from
56d9f55 to
c013a99
Compare
=== Do not change lines below ===
{
"chain": [],
"cmd": "./.update-readme-help.py",
"exit": 0,
"extra_inputs": [],
"inputs": [],
"outputs": [],
"pwd": "."
}
^^^ Do not change lines above ^^^
| except subprocess.CalledProcessError as exc: # when session_id has no processes | ||
| lgr.debug("Error collecting sample: %s", str(exc)) | ||
| return None |
There was a problem hiding this comment.
| except subprocess.CalledProcessError as exc: # when session_id has no processes | |
| lgr.debug("Error collecting sample: %s", str(exc)) | |
| return None | |
| except subprocess.CalledProcessError as exc: | |
| lgr.debug("Could not collect sample (process may have finished): %s", str(exc)) | |
| return None |
I know its just debug, but since this will happen almost every run we should make it sound not scary to unsuspecting users ;)
There was a problem hiding this comment.
Ok
No need for str though
There was a problem hiding this comment.
Since its set to DEBUG, i dont see a problem leaving the exc in, it could be useful if ps fails for some other reason
There was a problem hiding this comment.
some suggestions, feel free to accept as you see fit and merge when ready.
FWIW https://blog.datalad.org/posts/intro-duct-tion/ will be out of date (uses --quiet) lets fix that before we release
Co-authored-by: Austin Macdonald <[email protected]>
|
Dang, is searched for it, I guess for silent not quiet. I will add deprecation warning when get to laptop |
Before this commit we could effectively disable logging by setting the level to CRITICAL, but that only works because there are no CRITICAL logs, and that isn't something we should expect users to know. This feature is required by the use case in the datalad blog post https://blog.datalad.org/posts/intro-duct-tion/
=== Do not change lines below ===
{
"chain": [],
"cmd": "./.update-readme-help.py",
"exit": 0,
"extra_inputs": [],
"inputs": [],
"outputs": [],
"pwd": "."
}
^^^ Do not change lines above ^^^
When we have logging, and it also goes to stderr, there is really no need for some dedicated "duct_print". Now everything can be controlled with -l and no additional --quiet is needed (good that it was not demoed in https://blog.datalad.org/ post).
I also added a few lgr.debug level statements.
I delayed setup of logging.basicConfig until
mainso it might be more coherent later on with desire to make this whole thing into a reusable Python moduleCloses #134