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

Factor out duplicate setIsPrimaryFlag from MeasureMergedCoaddSourcesTask and ProcessCoaddTask

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: pipe_tasks
    • Labels:
      None
    • Story Points:
      2
    • Sprint:
      Science Pipelines DM-W16-6
    • Team:
      Data Release Production

      Description

      MeasureMergedCoaddSourcesTask.setIsPrimaryFlag() and ProcessCoaddTask.setIsPrimaryFlag() are effectively the same code. Please split this out into a separate task which both of the above can call.

      This is a (partial) port of HSC-1112 and should include fixes from HSC-1297.

        Attachments

          Issue Links

            Activity

            Hide
            rearmstr Bob Armstrong added a comment -

            Nate, can you review this for me?

            Show
            rearmstr Bob Armstrong added a comment - Nate, can you review this for me?
            Hide
            nlust Nate Lust added a comment -

            This all looks good to merge to me, as long as it passes Jenkins. Do you expect any of this to change outputs created by things in pipe tasks, as key names and such have changed? Or are the keys pretty much just used internally? If it will change test cases or if some ci stuff looks at the keys, please update there as well (to a reasonable amount of looking/greping for key names)

            Show
            nlust Nate Lust added a comment - This all looks good to merge to me, as long as it passes Jenkins. Do you expect any of this to change outputs created by things in pipe tasks, as key names and such have changed? Or are the keys pretty much just used internally? If it will change test cases or if some ci stuff looks at the keys, please update there as well (to a reasonable amount of looking/greping for key names)
            Hide
            rearmstr Bob Armstrong added a comment -

            I already passed it through jenkins. I tried to keep all the key names the same such that the outputs would be unchanged, but maybe I missed something? Did you see a key name that was different?

            Show
            rearmstr Bob Armstrong added a comment - I already passed it through jenkins. I tried to keep all the key names the same such that the outputs would be unchanged, but maybe I missed something? Did you see a key name that was different?
            Hide
            nlust Nate Lust added a comment -

            Nope, I didn't see anything, I just wanted to make sure you had thought about it and poked about a bit with say a search rather than just trusting ci. There may be something hidden in an if statement for instance that is not checked. Don't go crazy checking, but if a simple search or grep doesn't turn up something I think you are fine to merge.

            Show
            nlust Nate Lust added a comment - Nope, I didn't see anything, I just wanted to make sure you had thought about it and poked about a bit with say a search rather than just trusting ci. There may be something hidden in an if statement for instance that is not checked. Don't go crazy checking, but if a simple search or grep doesn't turn up something I think you are fine to merge.
            Hide
            rearmstr Bob Armstrong added a comment -

            merged.

            Show
            rearmstr Bob Armstrong added a comment - merged.

              People

              • Assignee:
                rearmstr Bob Armstrong
                Reporter:
                swinbank John Swinbank
                Reviewers:
                Nate Lust
                Watchers:
                Bob Armstrong, John Swinbank, Nate Lust
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel