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

Fix data ID handling in ap_*

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: ap_pipe, ap_verify
    • Labels:
      None
    • Story Points:
      2
    • Sprint:
      AP S18-5, AP S18-6
    • Team:
      Alert Production

      Description

      During investigation of DM-12672, I discovered several bugs in how ap_pipe and ap_verify handled data IDs:

      • Most command-line tasks implicitly assume their run or runDataRef methods take a fully expanded data reference. This is provided by pipe.base.ArgumentParser, but ap_pipe's task runner inadvertently bypassed the data ID expansion. In practice, this meant ImageDifferenceTask didn't have all the information it expected.
      • The same bypass would have prevented ap_pipe from processing multiple datasets specified as visit=#, though visit=# ccd=1..62 would have still worked.
      • ap_verify always provided partial data references, because it assumed the run method (or, more precisely, its butler calls) would expand any unambiguous reference.
      • The tests in test_ingestion.py were based on a naive view of how data IDs work, and most of the outstanding issues with the tests can now be fixed.

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment -

            Hi John Parejko, since I ended up following almost none of your advice, would you be willing to review this ticket? It's about 140 lines between ap_pipe and ap_verify.

            Show
            krzys Krzysztof Findeisen added a comment - Hi John Parejko , since I ended up following almost none of your advice, would you be willing to review this ticket? It's about 140 lines between ap_pipe and ap_verify .
            Hide
            Parejkoj John Parejko added a comment -

            Thanks for moving some of that code into daf_persistence. Jim Bosch and/or Pim Schellart [X] might want to look at it, as a reference for a Gen3 desired feature. I also asked a related question on slack:
            https://lsstc.slack.com/archives/C3UCAEW3D/p1526069070000689

            Show
            Parejkoj John Parejko added a comment - Thanks for moving some of that code into daf_persistence. Jim Bosch and/or Pim Schellart [X] might want to look at it, as a reference for a Gen3 desired feature. I also asked a related question on slack: https://lsstc.slack.com/archives/C3UCAEW3D/p1526069070000689

              People

              • Assignee:
                krzys Krzysztof Findeisen
                Reporter:
                krzys Krzysztof Findeisen
                Reviewers:
                John Parejko
                Watchers:
                John Parejko, Krzysztof Findeisen
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel