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

Merge generic and HSC-specific config overrides in obs_subaru

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
    • Story Points:
      15
    • Epic Link:
    • Team:
      Data Release Production
    • Urgent?:
      No

      Description

      The harder part of removing suprimecam support is merging the config overrides in config/ with those in config/HSC, but that also brings some of the biggest gains in terms of reduction of complexity.

      It may be most useful to do this when putting config overrides directly in pipeline YAML definitions becomes a realistic possibility (e.g. because the CmdLineTask forms of some tasks are being retired). I do not think all of the general overrides in config/ belong in pipelines, but some of them are more about the DRP pipeline in general than they are about Subaru instruments, and those should move.

        Attachments

        1. modified.log
          2.69 MB
        2. obs_subaru_configs.sh
          7 kB
        3. v1228c000dif.png
          v1228c000dif.png
          25 kB
        4. v1228c000mod.png
          v1228c000mod.png
          289 kB
        5. v1229c000van.png
          v1229c000van.png
          288 kB
        6. vanilla.log
          2.69 MB

          Issue Links

            Activity

            Hide
            lskelvin Lee Kelvin added a comment -

            Update - the above Jenkins run failed due to an issue in pipe_tasks associated with DM-27008 which was pushed to master a few hours before submitting this ticket for a final Jenkins run. These changes have now been reversed on the master branch.

            I set off a new Jenkins run this morning after the above changes had been reverted overnight, and this successfully passed just now. I hope this now looks okay to merge to master.

            Show
            lskelvin Lee Kelvin added a comment - Update - the above Jenkins run failed due to an issue in pipe_tasks associated with DM-27008 which was pushed to master a few hours before submitting this ticket for a final Jenkins run. These changes have now been reversed on the master branch. I set off a new Jenkins run this morning after the above changes had been reverted overnight, and this successfully passed just now. I hope this now looks okay to merge to master.
            Hide
            lskelvin Lee Kelvin added a comment -

            A number of minor edits made following comments on GitHub. To be safe, I've set off what I hope to be a final Jenkins run here. If all is well, I'll merge to master later on this evening.

            Show
            lskelvin Lee Kelvin added a comment - A number of minor edits made following comments on GitHub. To be safe, I've set off what I hope to be a final Jenkins run here . If all is well, I'll merge to master later on this evening.
            Hide
            Parejkoj John Parejko added a comment - - edited

            I'm sorry I didn't see this earlier: I definitely suggest not having the "generic" BVRI filter names in the filterMap. Those filters do not exist for HSC, so anything that would try to use them would be a bug and should fail. On this commit, I explicitly removed the config/jointcal.py reference that had been pulling those filters in.

            The same thing should apply to CalibrateTask configs as for jointcal: BVRI are not HSC filters, so should not exist in the filterMap at all.

            Show
            Parejkoj John Parejko added a comment - - edited I'm sorry I didn't see this earlier: I definitely suggest not having the "generic" BVRI filter names in the filterMap. Those filters do not exist for HSC, so anything that would try to use them would be a bug and should fail. On this commit , I explicitly removed the config/jointcal.py reference that had been pulling those filters in. The same thing should apply to CalibrateTask configs as for jointcal: BVRI are not HSC filters, so should not exist in the filterMap at all.
            Hide
            lskelvin Lee Kelvin added a comment -

            That's fine, I haven't pushed this yet and can easily remove them if you think it best. As the above Jenkins passed without issue, I'll remove these extra generic keys and push to master later this evening unless I hear otherwise in the next few hours.

            Show
            lskelvin Lee Kelvin added a comment - That's fine, I haven't pushed this yet and can easily remove them if you think it best. As the above Jenkins passed without issue, I'll remove these extra generic keys and push to master later this evening unless I hear otherwise in the next few hours.
            Hide
            lskelvin Lee Kelvin added a comment -

            Thanks again all for the comments everyone. An extra final Jenkins run completed successfully this evening. Branch merged to master and deleted.

            Show
            lskelvin Lee Kelvin added a comment - Thanks again all for the comments everyone. An extra final Jenkins run completed successfully this evening. Branch merged to master and deleted.

              People

              Assignee:
              lskelvin Lee Kelvin
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Lauren MacArthur
              Watchers:
              Jim Bosch, John Parejko, Lauren MacArthur, Lee Kelvin, Tim Jenness, Yusra AlSayyad
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.