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

setConfigRoot sometimes needs to not update the root

    Details

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

      Description

      Dino Bektesevic is running into problems with Butler.makeRepo whereby the registry specified from his external config (which happens to be an in-memory sqlite) is being overwritten by a default value assuming a SQLite file on disk with butlerRoot.

      I think it is reasonable to assume that if makeRepo is being called with a populated config and that that config has registry.root defined, that the caller of makeRepo does not want makeRepo to override the values. Should makeRepo attempt to create the tables in the provided root? What about datastore.root?

      It's a fairly small change to allow setConfigRoot to not override root if root is already present in config (we have to call it anyhow because it copies items over as well as updating the root). I think a flag to setConfigRoot ("overrideRoot"?) defaulting to False would work which would be True when called from makeRepo and would be forwarded to overrideParameters or else not even pass toUpdate to overrideParameters.

        Attachments

          Activity

          Hide
          tjenness Tim Jenness added a comment -

          Please take a look at this. I've pushed everything out into a boolean that defaults to the current behavior but allows people to ask for overrides to be retained when they use makeButlerRepo.

          Show
          tjenness Tim Jenness added a comment - Please take a look at this. I've pushed everything out into a boolean that defaults to the current behavior but allows people to ask for overrides to be retained when they use makeButlerRepo.
          Hide
          salnikov Andy Salnikov added a comment -

          Looks OK to me, maybe slightly confusing (I guess due to lack of exposure to Config classes). Few comments on PR.

          Show
          salnikov Andy Salnikov added a comment - Looks OK to me, maybe slightly confusing (I guess due to lack of exposure to Config classes). Few comments on PR.
          Hide
          tjenness Tim Jenness added a comment -

          Merged. I hope this helps people testing things out.

          Show
          tjenness Tim Jenness added a comment - Merged. I hope this helps people testing things out.

            People

            • Assignee:
              tjenness Tim Jenness
              Reporter:
              tjenness Tim Jenness
              Reviewers:
              Andy Salnikov
              Watchers:
              Andy Salnikov, Dino Bektesevic, Jim Bosch, Michelle Gower, Tim Jenness
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel