Attention is currently required from: dexter.
fixeria has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/41812?usp=email )
Change subject: transport/init: use PySimLogger to print messages ......................................................................
Patch Set 1:
(3 comments)
File pySim/transport/__init__.py:
https://gerrit.osmocom.org/c/pysim/+/41812/comment/15d4aabd_75e6313c?usp=ema... : PS1, Line 34: TRANSPORT I am wondering why are you doing this instead of using `__name__`. This would match the module hierarchy, e.g. for this module it would be `pySim.transport`. And this makes logging configuration flexible, allowing to set logging level for specific module(s). For instance, one could set `pySim.transport` to `DEBUG`, resulting in all modules in this directory to use thing logging level. This is exactly what the official documentation (https://docs.python.org/3/library/logging.html#logger-objects) recommends doing.
Also, with this approach it's not needed to do `logger.addHandler()` for each and every logger (this is what `PySimLogger.get` does) because child loggers do inherit logging configuration of the parent loggers. This enables other applications using pySim as a library to have their own logging configuration (formatting, handlers, etc.).
https://gerrit.osmocom.org/c/pysim/+/41812/comment/750862b2_f0053b1f?usp=ema... : PS1, Line 49: log.info Please use lazy format string rendering here and below.
https://gerrit.osmocom.org/c/pysim/+/41812/comment/14b3eee7_0b0c5cb1?usp=ema... : PS1, Line 368: os.environ.get('PYSIM_INTEGRATION_TEST') This workaround can be removed in a follow-up patch, thanks to proper logging.