debug and file log handler support for hyperion-blueapi#1630
debug and file log handler support for hyperion-blueapi#1630
hyperion-blueapi#1630Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1630 +/- ##
==========================================
+ Coverage 92.86% 92.87% +0.01%
==========================================
Files 153 154 +1
Lines 8645 8661 +16
==========================================
+ Hits 8028 8044 +16
Misses 617 617
🚀 New features to boost your workflow:
|
DominicOram
left a comment
There was a problem hiding this comment.
Thanks, one main comment. Additionally, the original issue says:
Decide whether we want debug and/or file logging, and if so that hyperion-blueapi supports them
Was the conversation had about dropping some of this support? I'm not sure how much the (non-debug) file is used given it's all in graylog anyway?
Also, does this close #1208? I guess it also relates to #1213?
| ] | ||
|
|
||
|
|
||
| def load_centre_collect( |
There was a problem hiding this comment.
Nit: I think it would be fine to not have the wrapper here for some of these plans and just add the inject into the plan itself. My concern is that e.g. the we change behaviour and update the docstring of the underlying plan but forget to update here
There was a problem hiding this comment.
I think that #1564 would complicate this somewhat since at least for LoadCentreCollect the internal parameters are not serializable; that PR creates a separate external parameter type which essentially makes this wrapper mandatory.
…that import has side-effects
|
As per discussion - I find the file logging useful, at any rate.
With regards to #1213 this doesn't address that yet - as far as this PR goes it is squarely aimed at getting hyperion-blueapi to functional parity with the non-blueapi hyperion, and doesn't aim to cater for k8s deployment. Although I think in most cases the debug log output would probably fit in a typical kubernetes volume since it is persisted on error. As for #1208 I guess it partially addresses this, at least in the non-k8s case |
Fixes #1377
Adds debug and file log handler support for
hyperion-blueapiThe behaviour of LOG_DIR and DEBUG_LOG_DIR has changed slightly - previously if not specified then the log file location would be calculated from the
BEAMLINEenvironment variable as/dls_sw/<beamline>logs/blueskyand/dls/tmp/<beamline>/logs/blueskyrespectively, now it requires both environment variables to be specified explicitly.The
run_hyperion.shlauncher script has been updated accordingly which should result in no change in normal usage, however this should reduce the potential to accidentally write to the wrong file locations in test.Link to dodal PR (if required): #N/A
(remember to update
pyproject.tomlwith the dodal commit tag if you need it for tests to pass!)Instructions to reviewer on how to test:
Checks for reviewer