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

Port ci_hsc to log package

    Details

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

      Description

      Switch from using pex_logging to log in ci_hsc.

        Attachments

          Issue Links

            Activity

            Hide
            swinbank John Swinbank added a comment -

            Hey Hsin-Fang Chiang

            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!

            Show
            swinbank John Swinbank added a comment - Hey Hsin-Fang Chiang — 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!
            Hide
            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 Perry Gee can say more what he saw?

            Show
            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 Perry Gee can say more what he saw?
            Hide
            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.

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

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

            Show
            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

              • Assignee:
                pgee Perry Gee
                Reporter:
                hchiang2 Hsin-Fang Chiang
                Reviewers:
                John Swinbank
                Watchers:
                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:

                  Summary Panel