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

Fix module versions

    Details

    • Team:
      Data Release Production

      Description

      Furusawa-san points out that "pipe_drivers" isn't in the packages provenance list. I believe this is because the __init__.py doesn't from .versions import *, so there's no version information to grab.

      I should survey other packages as well, and/or make a workaround in the code that saves the versions.

        Attachments

          Issue Links

            Activity

            Hide
            price Paul Price added a comment -

            Found several packages with the missing important line.

            Show
            price Paul Price added a comment - Found several packages with the missing important line.
            Hide
            price Paul Price added a comment -

            I added a test in ci_hsc to ensure the versions are getting set, and it's gone through successfully.

            Show
            price Paul Price added a comment - I added a test in ci_hsc to ensure the versions are getting set, and it's gone through successfully .
            Hide
            price Paul Price added a comment -

            Thanks for agreeing to review this, Chris Morrison. I hope it won't be a problem: despite the large number of packages, the changes are generally fairly simple.

            Show
            price Paul Price added a comment - Thanks for agreeing to review this, Chris Morrison . I hope it won't be a problem: despite the large number of packages, the changes are generally fairly simple.
            Hide
            cmorrison Chris Morrison added a comment - - edited

            Hi Paul,

            I've "thumbs'ed up" most of the changes as they were, like you said fairly straight forward. On the base and pipe_base packages could you provide me a few comments as to the context of what you are doing in these two packages? You made a comment above as to what is going on with ci_hsc and it would be helpful for me to get a little more understanding of what is being done with base and pipe_base and how it relates to this ticket. It seems to be more involved than just adding version information.

            Also your pull request on base doesn't have the ticket number in the title of the request.

            Show
            cmorrison Chris Morrison added a comment - - edited Hi Paul, I've "thumbs'ed up" most of the changes as they were, like you said fairly straight forward. On the base and pipe_base packages could you provide me a few comments as to the context of what you are doing in these two packages? You made a comment above as to what is going on with ci_hsc and it would be helpful for me to get a little more understanding of what is being done with base and pipe_base and how it relates to this ticket. It seems to be more involved than just adding version information. Also your pull request on base doesn't have the ticket number in the title of the request.
            Hide
            price Paul Price added a comment -

            I've fixed the base PR title.

            A bit more explanation:

            • base: The Packages class has a dict of packages (mapping package name to version). For testing, it's convenient to be able to iterate through the elements of Packages and to ask if a particular package is in the Packages, so I've added __contains__ and __iter__ methods to Packages that redirect to the dict that it holds.
            • pipe_base: This looks straightforward; did you mean a different package? Oh, I see pipe_tasks has some more stuff, so I'll assume you meant that.
            • pipe_tasks: This has the usual import addition, and I took the opportunity to add a file to .gitignore that's been bugging me for a while. But the more interesting change is to makeSkyMap.py, which wasn't persisting version information. That's because the MakeSkyMapRunner.precall was disabled, in order that it wouldn't persist the config or metadata. I changed that so that it now uses the standard way of disabling the config and metadata, which means that the standard precall fires, which checks and persists the version information (and I know it does because skymap is checked in ci_hsc).
            Show
            price Paul Price added a comment - I've fixed the base PR title. A bit more explanation: base : The Packages class has a dict of packages (mapping package name to version). For testing, it's convenient to be able to iterate through the elements of Packages and to ask if a particular package is in the Packages , so I've added __contains__ and __iter__ methods to Packages that redirect to the dict that it holds. pipe_base : This looks straightforward; did you mean a different package? Oh, I see pipe_tasks has some more stuff, so I'll assume you meant that. pipe_tasks : This has the usual import addition, and I took the opportunity to add a file to .gitignore that's been bugging me for a while. But the more interesting change is to makeSkyMap.py , which wasn't persisting version information. That's because the MakeSkyMapRunner.precall was disabled, in order that it wouldn't persist the config or metadata. I changed that so that it now uses the standard way of disabling the config and metadata, which means that the standard precall fires, which checks and persists the version information (and I know it does because skymap is checked in ci_hsc ).
            Hide
            cmorrison Chris Morrison added a comment -

            Yeah, I meant pipe_tasks. Okay, that seems fairly straight forward. Assuming it passes Jenkins then things should be good to go.

            Show
            cmorrison Chris Morrison added a comment - Yeah, I meant pipe_tasks. Okay, that seems fairly straight forward. Assuming it passes Jenkins then things should be good to go.
            Hide
            cmorrison Chris Morrison added a comment -

            Finished review. No significant changes needed.

            Show
            cmorrison Chris Morrison added a comment - Finished review. No significant changes needed.
            Hide
            price Paul Price added a comment -

            Thanks Chris.

            Merged to master.

            Show
            price Paul Price added a comment - Thanks Chris. Merged to master.

              People

              • Assignee:
                price Paul Price
                Reporter:
                price Paul Price
                Reviewers:
                Chris Morrison
                Watchers:
                Chris Morrison, Paul Price
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel