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

butler._addParents(self, repoDataList) ignores mapperArgs

    Details

    • Type: Story
    • Status: Invalid
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: butler
    • Labels:
      None
    • Team:
      Data Access and Database

      Description

      When passing arguments such as -calibRoot XXX to a command such as createCalibs.py it is ignored when the butler calls _addParents. I don't understand exactly what this call is doing, but it fails to pass the calibRoot down with the result that the butler gets instantiated using the default calibRoot which is incorrect.

      The obvious fix is to change line 855 of butler.py (in _addParents) to:

                      args = RepositoryArgs(cfgRoot=repoParentCfg.root, mode='r',
                                            mapperArgs=repoData.repoArgs.mapperArgs)
      

      although I think that there's another similar bug that I haven't yet had a chance to get to the bottom of. I'll edit this ticket when I get a chance.

      The how to repeat should be the same as DM-11284 but I haven't had a chance to check.

        Attachments

          Issue Links

            Activity

            Hide
            swinbank John Swinbank added a comment -

            This is a DAX/Butler issue. Paging Fritz Mueller & Nate Pease.

            Show
            swinbank John Swinbank added a comment - This is a DAX/Butler issue. Paging Fritz Mueller & Nate Pease .
            Hide
            rhl Robert Lupton added a comment -

            I looked a bit harder at this, and am a bit confused as there are two places that the butler keeps mapperArgs, and they are not always in sync, e.g.

            (pdb) w
            ...
              /Users/rhl/LSST/obs/base/python/lsst/obs/base/cameraMapper.py(826)_setupRegistry()
            -> registry = dafPersist.PosixRegistry(storage.root)
            (Pdb) print repoData.repoArgs.mapperArgs
            {'calibRoot': '/datasets/PFS/LAM/XCALIB'}
            (Pdb) print repoData.cfg.mapperArgs
            {}
            

            This means that my proposed fix may or may not be correct (should I use .cfg. or .repoArgs.?).

            Furthermore, even with this change we still get into trouble with repoData.cfg.mapperArgs being set to an empty dictionary; I think that the problem is

                        for repoParentIdx, repoParent in enumerate(repoData.cfg.parents):
                            parentIdxInRepoDataList = repoDataIdx + repoParentIdx + 1
                            if not isinstance(repoParent, RepositoryCfg):
                                args = RepositoryArgs(cfgRoot=repoParent, mode='r')
            

            If you change that to

                        for repoParentIdx, repoParent in enumerate(repoData.cfg.parents):
                            parentIdxInRepoDataList = repoDataIdx + repoParentIdx + 1
                            if not isinstance(repoParent, RepositoryCfg):
                                args = RepositoryArgs(cfgRoot=repoParent, mode='r', mapperArgs=repoData.cfg.mapperArgs)
            

            my problems go away as the block around line 855 doesn't fire. This makes sense to me as I don't understand why we need a new repoData, but I don't fully understand the logic by which you are adding new repos anyway.

            Show
            rhl Robert Lupton added a comment - I looked a bit harder at this, and am a bit confused as there are two places that the butler keeps mapperArgs , and they are not always in sync, e.g. (pdb) w ... /Users/rhl/LSST/obs/base/python/lsst/obs/base/cameraMapper.py(826)_setupRegistry() -> registry = dafPersist.PosixRegistry(storage.root) (Pdb) print repoData.repoArgs.mapperArgs {'calibRoot': '/datasets/PFS/LAM/XCALIB'} (Pdb) print repoData.cfg.mapperArgs {} This means that my proposed fix may or may not be correct (should I use .cfg. or .repoArgs. ?). Furthermore, even with this change we still get into trouble with repoData.cfg.mapperArgs being set to an empty dictionary; I think that the problem is for repoParentIdx, repoParent in enumerate(repoData.cfg.parents): parentIdxInRepoDataList = repoDataIdx + repoParentIdx + 1 if not isinstance(repoParent, RepositoryCfg): args = RepositoryArgs(cfgRoot=repoParent, mode='r') If you change that to for repoParentIdx, repoParent in enumerate(repoData.cfg.parents): parentIdxInRepoDataList = repoDataIdx + repoParentIdx + 1 if not isinstance(repoParent, RepositoryCfg): args = RepositoryArgs(cfgRoot=repoParent, mode='r', mapperArgs=repoData.cfg.mapperArgs) my problems go away as the block around line 855 doesn't fire. This makes sense to me as I don't understand why we need a new repoData , but I don't fully understand the logic by which you are adding new repos anyway.
            Hide
            rhl Robert Lupton added a comment -

            I have pushed a set of changes (relating to DM-11288 and DM-11289) to branch u/rhl/DM-11288

            Show
            rhl Robert Lupton added a comment - I have pushed a set of changes (relating to DM-11288 and DM-11289 ) to branch u/rhl/ DM-11288
            Hide
            npease Nate Pease added a comment -

            The 'problem' described in this issue was not a problem, it was written that way intentionally; the RepoArgs that are created & cited in the bug, using only the cfgRoot path and the rw mode, are used to load an existing RepositoryCfg, and so should not be populating any of the fields that would be used to create a cfg (e.g. mapperArgs). The problem is/was that the loading of Old Butler parent repositories was not being handled correctly, because the use of the repo when it was being used as an `input` was not recorded properly.
            This will be fixed in DM-11284: when an Old Butler repo is used as an `input`, its `RespositoryCfg` is nested in the parents list of the `output` repositories' `RepositoryCfg`.

            Show
            npease Nate Pease added a comment - The 'problem' described in this issue was not a problem, it was written that way intentionally; the RepoArgs that are created & cited in the bug, using only the cfgRoot path and the rw mode, are used to load an existing RepositoryCfg, and so should not be populating any of the fields that would be used to create a cfg (e.g. mapperArgs). The problem is/was that the loading of Old Butler parent repositories was not being handled correctly, because the use of the repo when it was being used as an `input` was not recorded properly. This will be fixed in DM-11284 : when an Old Butler repo is used as an `input`, its `RespositoryCfg` is nested in the parents list of the `output` repositories' `RepositoryCfg`.

              People

              • Assignee:
                Unassigned
                Reporter:
                rhl Robert Lupton
                Watchers:
                John Swinbank, Nate Pease, Robert Lupton
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel