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

Change ap_verify CI packages to use Butler.transfer_from

    XMLWordPrintable

    Details

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

      Description

      ap_verify_ci_cosmos_pdr2 and related packages use Butler.ingest in scripts/generate_refcats_gen3.py. It looks like these scripts are doing a butler to butler copy and therefore should be using Butler.transfer_from.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            The motivation for this ticket is the deprecation of unresolved DatasetRef and the change to Butler.ingest to now required resolved refs (DM-38779). Whilst investigating the ingest changes I realized that this code here is doing a butler-to-butler transfer so rather than fixing the ingest code it would be better to modernize the butler usage.

            Show
            tjenness Tim Jenness added a comment - The motivation for this ticket is the deprecation of unresolved DatasetRef and the change to Butler.ingest to now required resolved refs ( DM-38779 ). Whilst investigating the ingest changes I realized that this code here is doing a butler-to-butler transfer so rather than fixing the ingest code it would be better to modernize the butler usage.
            Hide
            krzys Krzysztof Findeisen added a comment -

            Hi Tim Jenness, would you be willing to review ap_verify_ci_cosmos_pdr2 and ap_verify_ci_hits2015? Excluding the preloaded repo and export.yaml, it's about 60 lines of actual code changes in either package.

            I chose not to update ap_verify_hits2015, as we will almost certainly never run the script before implementing DM-36477. ap_verify_ci_dc2 and ap_verify_testdata do not use the deprecated DatasetRef constructor and needed no changes.

            Show
            krzys Krzysztof Findeisen added a comment - Hi Tim Jenness , would you be willing to review ap_verify_ci_cosmos_pdr2 and ap_verify_ci_hits2015 ? Excluding the preloaded repo and export.yaml , it's about 60 lines of actual code changes in either package. I chose not to update ap_verify_hits2015 , as we will almost certainly never run the script before implementing DM-36477 . ap_verify_ci_dc2 and ap_verify_testdata do not use the deprecated DatasetRef constructor and needed no changes.
            Hide
            tjenness Tim Jenness added a comment -

            Looks good. I did run the code yesterday in earlier form. I will get DM-38870 out for review. I made a couple of comments on the PR that are possible additional simplifications.

            Show
            tjenness Tim Jenness added a comment - Looks good. I did run the code yesterday in earlier form. I will get DM-38870 out for review. I made a couple of comments on the PR that are possible additional simplifications.
            Hide
            krzys Krzysztof Findeisen added a comment -

            I forgot to port over the changes to ap_verify_dataset_template; that's now done. The changes to the template were a lot simpler because it had already dropped the practice of renaming refcats.

            Show
            krzys Krzysztof Findeisen added a comment - I forgot to port over the changes to ap_verify_dataset_template ; that's now done. The changes to the template were a lot simpler because it had already dropped the practice of renaming refcats.
            Hide
            krzys Krzysztof Findeisen added a comment -

            I just realized that it's better if I go ahead and merge now instead of waiting for DM-38870. Rebasing DM-38449 and DM-38450 onto the changes to the preloaded repo will be painful.

            Show
            krzys Krzysztof Findeisen added a comment - I just realized that it's better if I go ahead and merge now instead of waiting for DM-38870 . Rebasing DM-38449 and DM-38450 onto the changes to the preloaded repo will be painful.

              People

              Assignee:
              krzys Krzysztof Findeisen
              Reporter:
              tjenness Tim Jenness
              Reviewers:
              Tim Jenness
              Watchers:
              Krzysztof Findeisen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.