Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-17651

Forward lsst.log to Python logging

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • log
    • None
    • 2
    • Architecture

    Description

      When writing unit tests it is sometimes useful to check that a log message was created. Python provides TestCase.assertLogs for this purpose but this does not work with lsst.log. It would be useful to allow unit tests to temporarily forward lsst.log messages to Python logging so that they could be tested.

      This would have to ensure that DM-15201 does not cause a logging loop.

      Ideally C++ log4cxx messages could be forwarded but it seems much easier to focus on Python lsst.log messages.

      Attachments

        Activity

          tjenness Tim Jenness added a comment -

          salnikov I've rewritten the PR a bit since you looked at it on Friday. I realized that I do not need to raise an exception if forwarding is enabled and the LogHandler is used. All I really need to do is return immediately and let other Handlers work. I have attempted to do a check to see if other handlers will trigger and if I don't find any I send to standard output. Sometimes this leads to messages disappearing since I think a handler has been specified but nothing happens. The advice should probably be that if you are turning on log forwarding for testing that you should also add an explicit StreamHandler to the Python root logger. If you do that everything works fine.

          tjenness Tim Jenness added a comment - salnikov I've rewritten the PR a bit since you looked at it on Friday. I realized that I do not need to raise an exception if forwarding is enabled and the LogHandler is used. All I really need to do is return immediately and let other Handlers work. I have attempted to do a check to see if other handlers will trigger and if I don't find any I send to standard output. Sometimes this leads to messages disappearing since I think a handler has been specified but nothing happens. The advice should probably be that if you are turning on log forwarding for testing that you should also add an explicit StreamHandler to the Python root logger. If you do that everything works fine.
          tjenness Tim Jenness added a comment -

          One more thing, testing this is extremely hard because pytest adds a log handler itself and TestCase.assertLogs temporarily replaces all the handlers on the logger with a special handler. This means that you can get something that works when you run from the command line but fails in pytest (and vice versa).

          tjenness Tim Jenness added a comment - One more thing, testing this is extremely hard because pytest adds a log handler itself and TestCase.assertLogs temporarily replaces all the handlers on the logger with a special handler. This means that you can get something that works when you run from the command line but fails in pytest (and vice versa).

          Approved, few minor comments on PR. I tried to think about all possible combinations of loggers/handlers but my head exploded, maybe comments there don't make much sense.

          salnikov Andy Salnikov added a comment - Approved, few minor comments on PR. I tried to think about all possible combinations of loggers/handlers but my head exploded, maybe comments there don't make much sense.
          tjenness Tim Jenness added a comment -

          Regarding head exploding, I agree with you completely. I fiddled with this over the weekend and it was relatively easy to do a proof of concept but testing it proved to be really difficult with the interaction of pytest and unittest. I came to the conclusion that LogHandler being a no-op and requiring people to add an explicit additional root logger handler if they are doing the forwarding, made things much easier to think about.

          tjenness Tim Jenness added a comment - Regarding head exploding, I agree with you completely. I fiddled with this over the weekend and it was relatively easy to do a proof of concept but testing it proved to be really difficult with the interaction of pytest and unittest. I came to the conclusion that LogHandler being a no-op and requiring people to add an explicit additional root logger handler if they are doing the forwarding, made things much easier to think about.

          I'm happy with anything that works for unittest/pytest for now. Even if we need something more generic we'd have to limit ourselves to few specific use cases, otherwise chain reaction of head explosions is inevitable 

          salnikov Andy Salnikov added a comment - I'm happy with anything that works for unittest/pytest for now. Even if we need something more generic we'd have to limit ourselves to few specific use cases, otherwise chain reaction of head explosions is inevitable 
          tjenness Tim Jenness added a comment -

          Jenkins passes. Merged. Thanks for the review.

          tjenness Tim Jenness added a comment - Jenkins passes. Merged. Thanks for the review.

          People

            tjenness Tim Jenness
            tjenness Tim Jenness
            Andy Salnikov
            Andy Salnikov, Kian-Tat Lim, Tim Jenness
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Jenkins

                No builds found.