Attention is currently required from: dexter.

View Change

3 comments:

• File pySim/transport/__init__.py:

  • Patch Set #1, 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.).

  • Patch Set #1, Line 49: log.info

    Please use lazy format string rendering here and below.

  • Patch Set #1, Line 368: os.environ.get('PYSIM_INTEGRATION_TEST')

    This workaround can be removed in a follow-up patch, thanks to proper logging.

To view, visit change 41812. To unsubscribe, or for help writing mail filters, visit settings.