Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-789

Make lsst.log python interface more compatible with python logging

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Implemented
    • Resolution: Done
    • Component/s: DM, TCT
    • Labels:
      None

      Description

      The lsst.log python interface is very close to the python logging interface (by design) but there are a few areas where there are incompatibilities that make it harder for people to not care which logger they are using. These incompatibilities cause cognitive load on developers and make the implementation of RFC-782 a little more untidy.

      I would like to add the following to lsst.log

      • A name property (getName() as a property)
      • Adding the log.critical alias for log.fatal
      • Adding getEffectiveLevel as an alias for getLevel

      Python logging seems to prefer log.critical but has not actively deprecated log.fatal.

      Additionally, I would like to deprecate lsst.log usage of log.getLevel, log.warn (python has formally deprecated this in favor of log.warning), and log.getName() in the python interface. These deprecations are happening for Task as part of the implementation of RFC-782 but it will be easier for people if they are told if they are using something that might need to be changed later on.

      I'm hoping this is a fairly uncontroversial RFC given that we have long been moving towards always using python logging in Python code and lsst.log in C++ code and previously agreed that in RFC-782 and RFC-29.

        Attachments

          Issue Links

            Activity

            Hide
            jhoblitt Joshua Hoblitt added a comment -

            +1

            I would be in favor of going a step further and requiring the interface of lsst.log (as used for log messages) to be a subset of python logging to ensure compatibly.

            Show
            jhoblitt Joshua Hoblitt added a comment - +1 I would be in favor of going a step further and requiring the interface of lsst.log (as used for log messages) to be a subset of python logging to ensure compatibly.
            Hide
            tjenness Tim Jenness added a comment - - edited

            At this point I'd be in favor of reversing RFC-265 – only a handful of places adopted the API and Python logging went a completely different direction.

            As we migrate Task to use python logging the eventual endgame must surely be to switch all python code to python logging and use lsst.log in C++ code. We now have the tools to decide whether all python logging messages are forwarded to lsst.log (as is done now in command line task and pipetask) or to forward all the lsst.log messages to the python logging system (as recently made possible in DM-30996). This flexibility allows the general python code to assume python logging is being used and not care how the message is forwarded.

            Show
            tjenness Tim Jenness added a comment - - edited At this point I'd be in favor of reversing RFC-265 – only a handful of places adopted the API and Python logging went a completely different direction. As we migrate Task to use python logging the eventual endgame must surely be to switch all python code to python logging and use lsst.log in C++ code. We now have the tools to decide whether all python logging messages are forwarded to lsst.log (as is done now in command line task and pipetask) or to forward all the lsst.log messages to the python logging system (as recently made possible in DM-30996 ). This flexibility allows the general python code to assume python logging is being used and not care how the message is forwarded.
            Hide
            lguy Leanne Guy added a comment -

            +1 for " the eventual endgame must surely be to switch all python code to python logging "

            Show
            lguy Leanne Guy added a comment - +1 for " the eventual endgame must surely be to switch all python code to python logging "
            Hide
            rowen Russell Owen added a comment -

            +1. I'm in favor of making it as similar as reasonable. Should we add a {{level]} property, as well? (I've never been sure of the connection between the level property and getEffectiveLevel).

            Show
            rowen Russell Owen added a comment - +1. I'm in favor of making it as similar as reasonable. Should we add a {{level]} property, as well? (I've never been sure of the connection between the level property and getEffectiveLevel).
            Hide
            ktl Kian-Tat Lim added a comment -

            Should be OK; no objection.

            Show
            ktl Kian-Tat Lim added a comment - Should be OK; no objection.
            Hide
            tjenness Tim Jenness added a comment -

            The lsst.log.getLevel and the level property in logging are the same thing – they are the level that has been set for this specific logger. getEffectiveLevel takes into account parentage so if logger "a" is set to INFO but logger "a.b" has not level set, the level property for "a.b" would return -1 (in lsst.log) but getEffectiveLevel would return INFO because it would look at "a".

            Show
            tjenness Tim Jenness added a comment - The lsst.log.getLevel and the level property in logging are the same thing – they are the level that has been set for this specific logger. getEffectiveLevel takes into account parentage so if logger "a" is set to INFO but logger "a.b" has not level set, the level property for "a.b" would return -1 (in lsst.log) but getEffectiveLevel would return INFO because it would look at "a".

              People

              Assignee:
              tjenness Tim Jenness
              Reporter:
              tjenness Tim Jenness
              Watchers:
              Gregory Dubois-Felsmann, Ian Sullivan, Joshua Hoblitt, Kian-Tat Lim, Leanne Guy, Michelle Butler [X] (Inactive), Russell Owen, Tim Jenness, Wil O'Mullane, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  Jenkins

                  No builds found.