dictation-toolbox / natlinkcore

Other
4 stars 4 forks source link

Use named loggers instead of setting root logger in natlinkcore library #72

Closed LexiconCode closed 5 months ago

LexiconCode commented 5 months ago

This pr use named loggers instead of setting root logger in natlinkcore library. Fixing https://github.com/dictation-toolbox/natlinkcore/issues/71

Changes to loader.py

https://github.com/dictation-toolbox/natlinkcore/blob/2dd7df495cff67dc05e786666d268dfaa18763b1/src/natlinkcore/loader.py#L669-L699

LexiconCode commented 5 months ago

Test setting debug in config:

set log level to: DEBUG
Starting natlink loader from config file:
    "C:\Users\MainLaptop\.natlink\natlink.ini"
get_user_language, return userLanguage: "enx", (long language: "US English")
set_user_language, user: "MainLaptop", profile: "C:\ProgramData\Nuance\NaturallySpeaking13\Users\MainLaptop\current", language: "enx"
triggering load/reload process
loading module: _dragonfly_example_rule
WARNING:monitor.init:DPI awareness could not be set; it has been set already.
on_change_callback called with: "mic", "on"
triggering load/reload process
-on_begin_callback, new module info: ( (...)natspeak, Messages from Natlink, 3344720 )
triggering load/reload process
-on_begin_callback, same moduleInfo
on_change_callback called with: "mic", "off"

WARNING:monitor.init:DPI awareness could not be set; it has been set already. is from dragonfly.

LexiconCode commented 5 months ago

A few questions before merging.

From: docs.python.org/3/howto/logging.html#configuring-logging-for-a-library "Note It is strongly advised that you do not log to the root logger in your library. Instead, use a logger with a unique and easily identifiable name, such as the name for your library’s top-level package or module. Logging to the root logger will make it difficult or impossible for the application developer to configure the logging verbosity or handlers of your library as they wish.

Note It is strongly advised that you do not add any handlers other than NullHandler to your library’s loggers. This is because the configuration of handlers is the prerogative of the application developer who uses your library. The application developer knows their target audience and what handlers are most appropriate for their application: if you add handlers ‘under the hood’, you might well interfere with their ability to carry out unit tests and deliver logs which suit their requirements."

drmfinlay commented 5 months ago

I can answer these questions.

  • Should we be using NullHandler instead of Named logger for Natlink?

Loggers and logging handlers are different things. Natlink should be using a named logger as you have done in this PR. Natlink should not be using a NullHandler because it is not a library. Its log output should appear in the messages window. Hence, using StreamHandler is correct.

  • Could the logging could be simplified under one handler?

I don't think so. It looks like both logging handlers are important. One writes to stdout and one is used for remote debugging.