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

Forward lsst.log to Python logging

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: log
    • Labels:
      None
    • Story Points:
      2
    • Team:
      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

          Hide
          tjenness Tim Jenness added a comment -

          Andy 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.

          Show
          tjenness Tim Jenness added a comment - Andy 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.
          Hide
          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).

          Show
          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).
          Hide
          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.

          Show
          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.
          Hide
          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.

          Show
          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.
          Hide
          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 

          Show
          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 
          Hide
          tjenness Tim Jenness added a comment -

          Jenkins passes. Merged. Thanks for the review.

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

            People

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

              Dates

              Created:
              Updated:
              Resolved: