Skip to content

Conversation

@emmuhamm
Copy link
Contributor

@emmuhamm emmuhamm commented Feb 6, 2026

Description

Dependent on #37 going in. Alternatively, this can be merged into #37, after which that can go in.
Up to you Mr. Reviewer.

These have a few minor fixes that were discovered when implementing ERS into drunc and quality of life fixes

Changes

Three major changes are done:

Refactor logging demonstrator

Fixes #45

Logging demonstrator has become a behemoth of a monolith. This was a quick refactor that splits it off into various functions that that are all called in main. It should make things more readable and maintainable.

Fixes a streamhandler bug

Fixes #44

See above issue for description. This is now fixed by using the actual enums instead of strings to refer to things in the ERS.

Introduces better initialisation of the LHC

The LogHandlerConf by default initialises all the Base, Opmon, and ERS streams all at once. This is not always desired, because there are cases where we do want to initialise the base stream but not have to worry about ERS. This would be an issue because ERS requires the ERS env variables to be defined, when they are not always defined.

This fixes it by introducing a separate initialisation function, which is controlled by a new variable/argument of init_ers.
So the functionality is now as follows.

# Previous behaviour. Setting to true will initialise the ERS streams on the creation of handlerconf
# requires ERS envs to be defined
LHC_init = LogHandlerConf(init_ers=True)

# new behaviour (and now default!)
# By default init_ers is false
# When this is the case, ERS Streams are _not_ defined. Will survive without ERS envs being defined
LHC_no_init = LogHandlerConf() == LogHandlerConf(init_ers=False)

print(LHC_no_init.Base) # Success
print(LHC_no_init.ERS) # Throws ERS stream not initialised. Call init_ERS() first

# Later on, when ERS envs are defined, can be initialised
LHC_no_init.init_ERS() 
print(LHC_no_init.ERS) # Success

Reviewer test

  • Check out this branch on latest nightly
  • Run the logging demonstrator script with all the relevant options. Everything should work as expected.
  • For inspiration, run daqpytools-logging-demonstrator -r -f logger.log --ersprotobufstream --handlertypes --handlerconf -t -s
  • Check that:
    • the example messages print. there should be two (rich and stream)
    • An exception is the error and critical. There should be three (rich, stdout and stderr)
    • Throttle works. There should be two blocks, one is 40 msg, then 30s wait, then over 200 msg
    • Check handlertypes work. Messages should go where they say they go (rich goes to rich, file goes to file)
    • Check handlerconf works. They should go where they are directed to (see handler.py)
    • Check ers works. View on kafk
  • Feel free to run the above LHC ERS init to test as well

Type of change

  • Documentation (non-breaking change that adds or improves the documentation)
  • New feature or enhancement (non-breaking change which adds functionality)
  • Optimization (non-breaking change that improves code/performance)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (whatever its nature)

Testing checklist

  • Unit tests pass (e.g. dbt-build --unittest)
  • Minimal system quicktest passes (pytest -s minimal_system_quick_test.py)
  • Full set of integration tests pass (daqsystemtest_integtest_bundle.sh)
  • Python tests pass if applicable (e.g. python -m pytest)
  • Pre-commit hooks run successfully if applicable (e.g. pre-commit run --all-files)

Further checks

  • Code is commented where needed, particularly in hard-to-understand areas
  • Code style is correct (dbt-build --lint, and/or see https://dune-daq-sw.readthedocs.io/en/latest/packages/styleguide/)
  • If applicable, new tests have been added or an issue has been opened to tackle that in the future.
    (Indicate issue here: # (issue))

@emmuhamm emmuhamm force-pushed the emmuhamm/many-handlers-fixes branch from fd21c11 to d47ca1d Compare February 6, 2026 14:55
@emmuhamm emmuhamm self-assigned this Feb 9, 2026
@emmuhamm
Copy link
Contributor Author

emmuhamm commented Feb 10, 2026

The comment below has been deferred to #47

We also need one where a warning is thrown when this happens:

log = get_logger("name", stream_handler = True)

new_log = get_logger("name") # Will grab the correct logger

new_log_extra = get_logger("name", stream_handler=True, rich_handler=True) # this will get the correct logger, but not add the rich handler. This should be checked when performing this function and be made explicit..


@emmuhamm emmuhamm force-pushed the emmuhamm/many-handlers-fixes branch from 44a366a to ffdbf04 Compare February 11, 2026 15:57
@emmuhamm emmuhamm mentioned this pull request Feb 12, 2026
13 tasks
@emmuhamm emmuhamm force-pushed the emmuhamm/many-handlers-fixes branch from b20f442 to 29aeed6 Compare February 12, 2026 09:57
This was unlinked from issues Feb 12, 2026
@emmuhamm emmuhamm marked this pull request as ready for review February 12, 2026 10:39
@emmuhamm
Copy link
Contributor Author

Hi @PawelPlesniak, this is now ready for review :)

This is a dependency of DUNE-DAQ/drunc#660, so its important that this goes in before that one is tested.

Originally, this is supposed to go in after #37 goes in. However, since both are ready at the same time, I let you choose how you want to deploy this. Options are:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Refactor logging demonstrator [Bug]: StreamHandler log levels are weirdly being overridden

1 participant