Attention is currently required from: fixeria, laforge.

View Change

5 comments:

• Patchset:

  • Patch Set #1:

    Yes, I think using pythons logging framework is the better option here.

    Done

• Patchset:

  • Patch Set #6:

    It would be nice to have a command line option `-v/--verbose` to increase logging verbosity of pySim […]

    I have now added method to control the log level and the verbosity. In pySim-shell i have have now added a setable "verbose" that uses both methods in a simplified way. I also don't think that we need much here. In any case debugging will become a lot easier when we see the module name and the line number in the log.

• File pySim/log.py:

  • Patch Set #6, Line 62: staticmethod

    You can use the `@classmethod` instead, which allows to reference "this class". […]

    I think I have tried that, but _log_callback is passed to PySimLogHandler and this class may have problems with the cls parameter. I think I have tried that. At least @staticmethod worked better for me.

• File pySim/runtime.py:

  • Patch Set #6, Line 48: self.log = PySimLogger.get("RuntimeState")

    A more usual approach is to have a module-local logger: […]

    I have now changed it to a module local logger. Makes sense. I found out that we can always get the module name from the logger (%(module)s). The parameter we give to getLogger may be just an arbitrary name (%(name)s).

    So I think we should get the log using

    log = PySimLogger.get("ABC")

    "ABC" is then the log facility.

• File tests/pySim-trace_test/pySim-trace_test_gsmtap.pcapng.ok:

  • Patch Set #6, Line 9: USIM: a0000000871002

    is it intentional that we're loosing this output in the unit test? […]

    This was intentional with the previous approach (even though it was not a good idea, but I thought that those messages were dispensable in the pySim-trace usecase).

    Since the current approach should not alter the behavior of existing code by default we should not get the messages of course. However there still may be slight alterations. Insisting on not changing any output in the slightest way would require a lot of extra code in PySimLogger. At the moment we still lose the "warning: " prefix in one line. I think that this is acceptable.

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