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

Suggest logging migration in afw

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Suggest a changeset with lsst::log instead of pex::logging in afw

        Attachments

          Issue Links

            Activity

            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Ran a Jenkins just to double-check some pex_logging includes can be removed regardless of the logging framework migration:
            https://ci.lsst.codes/job/stack-os-matrix/label=centos-7,python=py2/14894//console

            Show
            hchiang2 Hsin-Fang Chiang added a comment - Ran a Jenkins just to double-check some pex_logging includes can be removed regardless of the logging framework migration: https://ci.lsst.codes/job/stack-os-matrix/label=centos-7,python=py2/14894//console
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Jim Bosch, may you please review this ticket that removes pex_logging dependency from afw?

            I find all loggers of distinct names in afw use no more than one Debug/Trace level, except (lsst.)afw.math, so I convert all debugging/tracing except afw.math to log at lsst.log DEBUG level. For afw.math with multiple tracing levels, I use the prefix (TRACEn.afw.math.component, where n=1-5) as discussed in RFC-203. afw.math loggers use Trace 3,4,5,6 in pex.logging and I converted them into TRACE2, TRACE3, TRACE4, and TRACE5 (one lower in the number).

            I tried to group different types of logging usages into different commits; hopefully that makes the conversion clearer.

            In some tests and examples there are debugging placeholders such as

            lsst::pex::logging::Trace::setVerbosity("afw.component", 5)
            lsst.pex.logging.Trace_setVerbosity("afw.component", 0)
            lsst.pex.logging.Debug("afw.componrnt", 0)
            

            Some of them seem outdated in the master because they set logging levels for non-existent loggers. I manually ran some tests and examples to see where the logging messages came from and updated the debugging placeholders if I was able to guess what the author meant originally.

            Jenkins passed.

            Besides afw, minor changes are done in the following packages for correcting the dependency.

            • shapelet, meas_base, meas_extensions_simpleShape, obs_subaru: Removing the pex_logging dependency in swig files.
            • pipe_base, meas_algorithms: correct the hidden pex_logging dependency in ups.
            Show
            hchiang2 Hsin-Fang Chiang added a comment - Jim Bosch , may you please review this ticket that removes pex_logging dependency from afw? I find all loggers of distinct names in afw use no more than one Debug/Trace level, except (lsst.)afw.math , so I convert all debugging/tracing except afw.math to log at lsst.log DEBUG level. For afw.math with multiple tracing levels, I use the prefix (TRACEn.afw.math.component, where n=1-5) as discussed in RFC-203 . afw.math loggers use Trace 3,4,5,6 in pex.logging and I converted them into TRACE2, TRACE3, TRACE4, and TRACE5 (one lower in the number). I tried to group different types of logging usages into different commits; hopefully that makes the conversion clearer. In some tests and examples there are debugging placeholders such as lsst::pex::logging::Trace::setVerbosity( "afw.component" , 5 ) lsst.pex.logging.Trace_setVerbosity( "afw.component" , 0 ) lsst.pex.logging.Debug( "afw.componrnt" , 0 ) Some of them seem outdated in the master because they set logging levels for non-existent loggers. I manually ran some tests and examples to see where the logging messages came from and updated the debugging placeholders if I was able to guess what the author meant originally. Jenkins passed. Besides afw , minor changes are done in the following packages for correcting the dependency. shapelet , meas_base , meas_extensions_simpleShape , obs_subaru : Removing the pex_logging dependency in swig files. pipe_base , meas_algorithms : correct the hidden pex_logging dependency in ups.
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            I created 7 PRs for the 7 packages. All but afw are small and only a few lines of changes. There are a few more ticket branches which were created for passing Jenkins before DM-6999/DM-6984 were merged, but they are no longer needed and are there only to ensure Jenkins pulls the master; please ignore those (e.g., log, daf_persistence, etc).

            Show
            hchiang2 Hsin-Fang Chiang added a comment - I created 7 PRs for the 7 packages. All but afw are small and only a few lines of changes. There are a few more ticket branches which were created for passing Jenkins before DM-6999 / DM-6984 were merged, but they are no longer needed and are there only to ensure Jenkins pulls the master; please ignore those (e.g., log, daf_persistence, etc).
            Hide
            jbosch Jim Bosch added a comment -

            Review complete. There are a few requested changes on the afw PR, and (as you predicted) nothing for any of the other packages.

            I'm sorry you had to spend time on this fixing up what's essentially dead code - as you've probably seen, this spurred some RFCs to remove that code so at least this won't happen again.

            Show
            jbosch Jim Bosch added a comment - Review complete. There are a few requested changes on the afw PR, and (as you predicted) nothing for any of the other packages. I'm sorry you had to spend time on this fixing up what's essentially dead code - as you've probably seen, this spurred some RFCs to remove that code so at least this won't happen again.
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Thank you Jim Bosch for reviewing (and thank you for those RFCs!) The requested changes on the afw PR have been done, including moving the generic tracing utility function to lsst.log and a new PR is opened there . May you please take a look there? The documentations are only in RFC-203 right now but I'll improve the documentations, probably including adding an example, in DM-7530.

            Show
            hchiang2 Hsin-Fang Chiang added a comment - Thank you Jim Bosch for reviewing (and thank you for those RFCs!) The requested changes on the afw PR have been done, including moving the generic tracing utility function to lsst.log and a new PR is opened there . May you please take a look there? The documentations are only in RFC-203 right now but I'll improve the documentations, probably including adding an example, in DM-7530 .
            Hide
            jbosch Jim Bosch added a comment -

            New PR looks fine, thanks!

            Show
            jbosch Jim Bosch added a comment - New PR looks fine, thanks!
            Hide
            hchiang2 Hsin-Fang Chiang added a comment -

            Merged the 8 PRs after running default Jenkins and ci_hsc. And a build of afw to show pex_logging is no longer needed in afw

            Show
            hchiang2 Hsin-Fang Chiang added a comment - Merged the 8 PRs after running default Jenkins and ci_hsc . And a build of afw to show pex_logging is no longer needed in afw

              People

              Assignee:
              hchiang2 Hsin-Fang Chiang
              Reporter:
              hchiang2 Hsin-Fang Chiang
              Reviewers:
              Jim Bosch
              Watchers:
              Hsin-Fang Chiang, Jim Bosch
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.