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

Switch to YamlStorage instead of BoostStorage in all obs packages

    XMLWordPrintable

    Details

    • Story Points:
      2
    • Sprint:
      BG3_F18_08
    • Team:
      Data Release Production

      Description

      Now that YamlStorage has been implemented, we should use it.

        Attachments

          Issue Links

            Activity

            Hide
            jbosch Jim Bosch added a comment -

            This is an almost-entirely-mechanical change that probably isn't worth reviewing in detail across the board.  To be specific, most packages have one commit with the message "Switch to YAML for PropertySet/PropertyList storage" - that was entirely a mechanical find-and-replace for:

            • BoostStorage -> YamlStorage
            • .boost -> .yaml

            ...and since I already looked at what that did, I don't think you should feel like you need to as well.  That is the only commit in most obs packages.

            These packages have more than that:

            • obs_base: updates to test code (in addition to a big mechanical commit)
            • ip_diffim, meas_astrom, meas_deblender: old, bitrotted, example code that I've either removed or minimally changed to avoid reference to BoostStorage.

            Looking at those would make this a pretty tiny review.

            Meredith Rawls, I'm sending this to you in part because I don't think I've sent a review to you before, and I'm trying to branch out a bit, and partly because I'm not sure if you're using the Task metadata in ap_pipe in a way that might exercise this change in ways that are different from what I've tested (lsst_distrib and ci_hsc).  If that's the case I was hoping to learn what else I should do to test that this does no harm.

            Show
            jbosch Jim Bosch added a comment - This is an almost-entirely-mechanical change that probably isn't worth reviewing in detail across the board.  To be specific, most packages have one commit with the message "Switch to YAML for PropertySet/PropertyList storage" - that was entirely a mechanical find-and-replace for: BoostStorage -> YamlStorage .boost -> .yaml ...and since I already looked at what that did, I don't think you should feel like you need to as well.  That is the only commit in most obs packages. These packages have more than that: obs_base : updates to test code (in addition to a big mechanical commit) ip_diffim , meas_astrom , meas_deblender : old, bitrotted, example code that I've either removed or minimally changed to avoid reference to BoostStorage. Looking at those would make this a pretty tiny review. Meredith Rawls , I'm sending this to you in part because I don't think I've sent a review to you before, and I'm trying to branch out a bit, and partly because I'm not sure if you're using the Task metadata in ap_pipe in a way that might exercise this change in ways that are different from what I've tested (lsst_distrib and ci_hsc).  If that's the case I was hoping to learn what else I should do to test that this does no harm.
            Hide
            mrawls Meredith Rawls added a comment -

            This change looks fine to me. My main question is, how will this affect existing repos that use BoostStorage? Will we still be able to access them with the Butler in the usual ways or will they be broken in some way? 

            Show
            mrawls Meredith Rawls added a comment - This change looks fine to me. My main question is, how will this affect existing repos that use BoostStorage? Will we still be able to access them with the Butler in the usual ways or will they be broken in some way? 
            Hide
            jbosch Jim Bosch added a comment -

            This would indeed break access to existing repos using BoostStorage. I thought a bit about trying to add some sort of backwards compatibility, but given that these files are so rarely accessed (and, in particular, are not to my knowledge ever used as the input to any actual pipeline or automated analysis code), I figured that just using an older version of the stack to read them would probably be sufficient.

            Show
            jbosch Jim Bosch added a comment - This would indeed break access to existing repos using BoostStorage. I thought a bit about trying to add some sort of backwards compatibility, but given that these files are so rarely accessed (and, in particular, are not to my knowledge ever used as the input to any actual pipeline or automated analysis code), I figured that just using an older version of the stack to read them would probably be sufficient.
            Hide
            mrawls Meredith Rawls added a comment -

            OK, that's good to know. To be fair, I have almost never accessed .boost files myself. Could you please write a short community post about the switch from a user perspective, addressing the scenario of someone trying to use the stack to access some Useful Information from BoostStorage for an old repo and how they could proceed. (Try to read the file with a text editor? Use an old stack [insert suggested weekly here]? Somehow convert it to YamlStorage after the fact? And any other options you either recommend or don't recommend.) There are a couple older "deboostification" community posts from an "under the hood" perspective, but they're not helpful for solving this new breakage.

            Show
            mrawls Meredith Rawls added a comment - OK, that's good to know. To be fair, I have almost never accessed .boost files myself. Could you please write a short community post about the switch from a user perspective, addressing the scenario of someone trying to use the stack to access some Useful Information  from BoostStorage for an old repo and how they could proceed. (Try to read the file with a text editor? Use an old stack [insert suggested weekly here] ? Somehow convert it to YamlStorage after the fact? And any other options you either recommend or don't recommend.) There are a couple older "deboostification" community posts from an "under the hood" perspective, but they're not helpful for solving this new breakage.
            Hide
            jbosch Jim Bosch added a comment -

            Will do; I was already planning to write a community post, and I'll be certain to make sure all of those points are covered.

            Show
            jbosch Jim Bosch added a comment - Will do; I was already planning to write a community post, and I'll be certain to make sure all of those points are covered.

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Meredith Rawls
              Watchers:
              Jim Bosch, Krzysztof Findeisen, Meredith Rawls
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.