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

Port ci_hsc to log package

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • ci_hsc
    • None
    • 2
    • DRP S17-1
    • Data Release Production

    Description

      Switch from using pex_logging to log in ci_hsc.

      Attachments

        Issue Links

          Activity

            Hey hchiang2

            I understood from the discussion above that it was necessary that all the dependencies of ci_hsc be converted to use pex_logging before it was possible to convert ci_hsc itself. However, I just tried running the tickets/DM-8356 branch of ci_hsc with the latest weekly (w51) of meas_modelfit which pre-dates the logging conversion, and it worked fine!

            Can you explain a bit what's going on here? Do dependencies need to be converted or not? How do the logging systems interact?

            Thanks!

            swinbank John Swinbank added a comment - Hey hchiang2 — I understood from the discussion above that it was necessary that all the dependencies of ci_hsc be converted to use pex_logging before it was possible to convert ci_hsc itself. However, I just tried running the tickets/ DM-8356 branch of ci_hsc with the latest weekly (w51) of meas_modelfit which pre-dates the logging conversion, and it worked fine! Can you explain a bit what's going on here? Do dependencies need to be converted or not? How do the logging systems interact? Thanks!

            I said it's possible (not necessary!) that dependent packages need to remove pex_logging first. From my experience, the dependency blocking sometimes happened in c++ codes; mostly it was afw and somehow I couldn't remove downstream pex_logging dependency in c++ while afw still used pex_logging. I suspected it was some SWIG thing but didn't follow through to know exactly why. If you are really curious I may go back to find an example.

            On second thought, it's indeed puzzling how meas_modelfit could block ci_hsc. There seems no log passing, and after all, ci_hsc is pure python, right? I didn't try anything practical here. Maybe pgee can say more what he saw?

            hchiang2 Hsin-Fang Chiang added a comment - I said it's possible (not necessary !) that dependent packages need to remove pex_logging first. From my experience, the dependency blocking sometimes happened in c++ codes; mostly it was afw and somehow I couldn't remove downstream pex_logging dependency in c++ while afw still used pex_logging. I suspected it was some SWIG thing but didn't follow through to know exactly why. If you are really curious I may go back to find an example. On second thought, it's indeed puzzling how meas_modelfit could block ci_hsc. There seems no log passing, and after all, ci_hsc is pure python, right? I didn't try anything practical here. Maybe pgee can say more what he saw?
            pgee Perry Gee added a comment -

            No, it's not pure python. It runs the pipeline, which calls C++ for most of its measurement algorithms. The problem was occurring in swig I believe, though it has been weeks, and I don't really recall. I never tracked down the smoking gun, nor did I feel motivated to do so when the problem when away. But I will try to reproduce it when I have the time. I may not have been in the build and setup state I thought I was.

            pgee Perry Gee added a comment - No, it's not pure python. It runs the pipeline, which calls C++ for most of its measurement algorithms. The problem was occurring in swig I believe, though it has been weeks, and I don't really recall. I never tracked down the smoking gun, nor did I feel motivated to do so when the problem when away. But I will try to reproduce it when I have the time. I may not have been in the build and setup state I thought I was.

            Change looks basically fine. Trivial comment on the PR.

            As a reminder, our procedure is that the author of the code makes the PR before they submit it for review. See e.g. here.

            swinbank John Swinbank added a comment - Change looks basically fine. Trivial comment on the PR . As a reminder, our procedure is that the author of the code makes the PR before they submit it for review. See e.g. here .

            Thanks both. I suspect that the problem Perry saw was just a coincidence, and wasn't actually related to the logging transition at all. Not worth spending more timing tracking it down.

            swinbank John Swinbank added a comment - Thanks both. I suspect that the problem Perry saw was just a coincidence, and wasn't actually related to the logging transition at all. Not worth spending more timing tracking it down.

            People

              pgee Perry Gee
              hchiang2 Hsin-Fang Chiang
              John Swinbank
              Hsin-Fang Chiang, John Swinbank, Lauren MacArthur, Merlin Fisher-Levine, Perry Gee
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.