Add ability to configure logger based on ERS configuration safely#49
Draft
Add ability to configure logger based on ERS configuration safely#49
Conversation
713f4a1 to
30ce383
Compare
30ce383 to
67123b9
Compare
This was referenced Feb 19, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes (No existing issue)
Context
During testing of drunc, it was identified that we should be able to configure the handlers of a logging instance based on the ERS config supplied. This posed a greater question of how we deal with configuring the initialisation of handlers for loggers in general. Keep in mind that so far the features of the LogHandlerConf are entirely used for filtering; this is completely separate compared to how the logging was initialised.
A couple of key points are identified:
extraargument was used to choose which Handler to useextra=HandlerType.Rich...in every messageThe changes here are meant to tackle the above.
Changes
Fallback Handlers
The concept of Fallback Handlers are introduced. The user experience is quite straight forward:
Test exceprt
All the details on how it works is in the annotated text above from a users perspective.
Technically speaking, the difference compared to before is found in replacing this line:
daqpytools/src/daqpytools/logging/handlers.py
Line 370 in 7b81745
And making it so we can modify it by hand.
And then we modify every single
add_x_handlerto change this value to whatever we want. And by default, it will set it to whatever the current handlertype is.For example,
add_rich_handlerwill make the default case aHandlerType.Rich.Slight refactoring of code
To get through the next bit of change, theres a slight refactoring of the code.
The most important development of which is the new function
add_handlers_from_types, which is a generic function that allows you to add whatever handler you want by submitting a (set of) HandlerTypes. This cleans up theget_daq_loggercode decently well, while making it quite adjustable.The bulk of the new code is based on having this
handler_configs, which is a dict that holds the HandlerType, the type(Handler) as seen by python, and the installer code that it has. This can then be looped to install as necessary.Something that requires a slight refactor is the code that checks if a logger has a handler or not. The code is now more general so that we can check if the current logger has it, or an ancestor. This is used several times in
add_handlers_from_types.With this, the new
add_handlers_from_typeswill check the loggers to see if any of the logger or any of the ancestors have that handler. If it does, it will not add it.Note, that while this is useful, it is now a bit bloated. See #51
Setup ERS Handlers
With the previous changes implemented, a new function called
setup_daq_ers_loggerhas been implemented.With everything set up, this is a relatively simple function. All it does is to get the oks configuration, gets a set of all HandlerTypes, and then passess it to
add_handlers_from_typesto get it added to the loggers.Importantly, the fallback_handlers for this is set to
HandlerType.Unknown. This means that none of the new handlers added by the function will not be called unless specifically requested for!logging demonstrator
Of course, the logging demonstrator has been updated.
Change topic name in ERS
Oh yeah I also fixed the ability to choose which session name the ERS handler goes to now.
Follow up
This PR, while useful, reveals how bloated the
handlers.pyfile is, and how theres too much mixing of responsibilities with handlers, filters, and loggers. These should be refactored to prevent bloat and work on maintainability.A description of suggested changes and suggested implementation has been added in #51. This will need to be tackled before any future developments can happen
Type of change
Key checklist
python -m pytest)pre-commit run --all-files)Further checks
(Indicate issue here: # (issue))