Tidy up i24 serial logs #572
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #572 +/- ##
==========================================
+ Coverage 78.67% 80.00% +1.32%
==========================================
Files 97 97
Lines 6848 6806 -42
==========================================
+ Hits 5388 5445 +57
+ Misses 1460 1361 -99
|
|
Deferred to after #573 is done and running on the beamline because of permission problems if run by staff/users |
| @@ -103,7 +106,7 @@ def config( | |||
| integrate_all_logs=False, | |||
| ) | |||
| # Remove dodal StreamHandler to avoid duplication of messages above debug | |||
| dodal_logger.removeHandler(dodal_logger.handlers[0]) | |||
| # dodal_logger.removeHandler(dodal_logger.handlers[0]) | |||
There was a problem hiding this comment.
should: remove this comment
There was a problem hiding this comment.
That was accidentally commented out, sorry, fixed it now
There was a problem hiding this comment.
Thanks, added some small comments. There are quite a few lines with no test coverage, but I will approve anyway as I know you're keen to test a release today.
A lot of the the untested lines are logging statements, I'd suggest testing them using by setting up logging in pytest_runtest_setup(item) in an i24 conftest, similiar to what we have in mx-bluesky/tests/conftest.py. Then we can easily assert logging calls after a@patch("mx_bluesky.beamlines.i24.serial.log.SSX_LOGGER")
| logger.addHandler(logging.NullHandler()) | ||
| logger.parent = dodal_logger | ||
| SSX_LOGGER = logging.getLogger("I24serial") | ||
| SSX_LOGGER.addHandler(logging.NullHandler()) |
There was a problem hiding this comment.
What's the NullHandler for?
| @@ -68,10 +58,8 @@ def _read_visit_directory_from_file() -> Path: | |||
|
|
|||
|
|
|||
| def _get_logging_file_path() -> Path: | |||
There was a problem hiding this comment.
I think we might have already discussed this, but I can't remember. How come this needs to be in a different location to that given by the common util function (/dls_sw/{beamline}/logs/bluesky/)?
There was a problem hiding this comment.
SO that users/staff can access it, this is the logger that looks like the old output they are used to. May not be needed for long, but for the moment it's useful for debugging
| # Open the edm screen for a fixed target serial collection | ||
| echo "Starting fixed target edm screen." | ||
| edm -x "${edm_path}/FT-gui/DiamondChipI24-py3v1.edl" | ||
|
|
||
| echo "Edm screen closed, bye!" |
There was a problem hiding this comment.
Well... it's three lines or so later :)
Closes #557