daf_persistence Tickets/dm 9039

python/lsst/daf/persistence/butlerSubset.py

@@ -240,22 +240,24 @@ def subItems(self, level=None):
         default lower level for the original ButlerSubset level and dataset
         type is used.
 
+        As currently implemented, the default sublevels for all the
+        repositories used by this Butler instance must match for the Butler to
+        be able to select a default sublevel to get the subset.
+
         @param level (str)   the hierarchy level to descend to.
         @returns (ButlerSubset) resulting from the lower-level query or () if
                                 there is no lower level.
         """
 
         if level is None:
-            mappers = []
-            for repoData in self.butlerSubset.butler._repos.all():
-                if repoData.repo._mapper not in mappers:
-                    mappers.append(repoData.repo._mapper)
-            if len(mappers) != 1:
-                raise RuntimeError("Support for multiple repositories not yet implemented!")
-            mapper = mappers[0]
-
-            # todo: getDefaultSubLevel is not in the mapper API!
-            level = mapper.getDefaultSubLevel(self.butlerSubset.level)
+            levelSet = set()
+            for repoData in self.butlerSubset.butler._repos.all().values():
+                levelSet.add(repoData.repo._mapper.getDefaultSubLevel(
+                             self.butlerSubset.level))
+            if len(levelSet) > 1:
+                raise RuntimeError(
+                    "Support for multiple levels not implemented.")
+            level = levelSet.pop()
             if level is None:
                 return ()
         return self.butlerSubset.butler.subset(self.butlerSubset.datasetType,

python/lsst/daf/persistence/posixStorage.py

@@ -71,7 +71,7 @@ def _getRepositoryCfg(uri):
             with open(loc, 'r') as f:
                 repositoryCfg = yaml.load(f)
             if repositoryCfg.root is None:
-                repositoryCfg.root = parseRes.path
+                repositoryCfg.root = uri
         return repositoryCfg
 
     @staticmethod
@@ -80,25 +80,10 @@ def getRepositoryCfg(uri):
         if repositoryCfg is not None:
             return repositoryCfg
 
-        # if no repository cfg, is it a legacy repository?
-        parseRes = urllib.parse.urlparse(uri)
-        if repositoryCfg is None:
-            mapper = PosixStorage.getMapperClass(parseRes.path)
-            if mapper is not None:
-                repositoryCfg = RepositoryCfg(mapper=mapper,
-                                              root=parseRes.path,
-                                              mapperArgs=None,
-                                              parents=None,
-                                              isLegacyRepository=True,
-                                              policy=None)
         return repositoryCfg
 
     @staticmethod
     def putRepositoryCfg(cfg, loc=None):
-        if cfg.isLegacyRepository:
-            # don't write cfgs to legacy repositories; they take care of themselves in other ways (e.g. by
-            # the _parent symlink)
-            return
         if loc is None or cfg.root == loc:
             # the cfg is at the root location of the repository so don't write root, let it be implicit in the
             # location of the cfg.
@@ -160,6 +145,32 @@ def getMapperClass(root):
 
         return None
 
+    @staticmethod
+    def getParentSymlinkPath(root):
+        """For Butler V1 Repositories only, if a _parent symlink exists, get the location pointed to by the
+        symlink.
+
+        Parameters
+        ----------
+        root : string
+            A path to the folder on the local filesystem.
+
+        Returns
+        -------
+        string or None
+            A path to the parent folder indicated by the _parent symlink, or None if there is no _parent
+            symlink at root.
+        """
+        linkpath = os.path.join(root, '_parent')
+        if os.path.exists(linkpath):
+            try:
+                return os.readlink(os.path.join(root, '_parent'))
+            except OSError:
+                # some of the unit tests rely on a folder called _parent instead of a symlink to aother
+                # location. Allow that; return the path of that folder.
+                return os.path.join(root, '_parent')
+        return None
+
     def mapperClass(self):
         """Get the class object for the mapper specified in the stored repository"""
         return PosixStorage.getMapperClass(self.root)
@@ -317,6 +328,25 @@ def lookup(self, *args, **kwargs):
         """Perform a lookup in the registry"""
         return self.registry.lookup(*args, **kwargs)
 
+    @staticmethod
+    def v1RepoExists(root):
+        """Test if a Version 1 Repository exists.
+
+        Version 1 Repositories only exist in posix storages and do not have a RepositoryCfg file.
+        To "exist" the folder at root must exist and contain files or folders.
+
+        Parameters
+        ----------
+        root : string
+            A path to a folder on the local filesystem.
+
+        Returns
+        -------
+        bool
+            True if the repository at root exists, else False.
+        """
+        return os.path.exists(root) and bool(os.listdir(root))
+
 
 Storage.registerStorageClass(scheme='', cls=PosixStorage)
 Storage.registerStorageClass(scheme='file', cls=PosixStorage)

python/lsst/daf/persistence/repository.py

@@ -34,26 +34,37 @@ class RepositoryArgs(object):
 
     def __init__(self, root=None, cfgRoot=None, mapper=None, mapperArgs=None, tags=None,
                  mode=None, policy=None):
-        self.root = root
+        self._root = root
         self._cfgRoot = cfgRoot
-        self.mapper = mapper
+        self._mapper = mapper
         self.mapperArgs = mapperArgs
         self.tags = set(listify(tags))
         self.mode = mode
         self.policy = Policy(policy) if policy is not None else None
-        self.isLegacyRepository = False
+
 
     def __repr__(self):
         return "%s(root=%r, cfgRoot=%r, mapper=%r, mapperArgs=%r, tags=%s, mode=%r, policy=%s)" % (
-            self.__class__.__name__, self.root, self._cfgRoot, self.mapper, self.mapperArgs, self.tags,
+            self.__class__.__name__, self.root, self._cfgRoot, self._mapper, self.mapperArgs, self.tags,
             self.mode, self.policy)
 
+    @property
+    def mapper(self):
+        return self._mapper
+
+    @mapper.setter
+    def mapper(self, mapper):
+        if mapper is not None and self._mapper:
+            raise RuntimeError("Explicity clear mapper (set to None) before changing its value.")
+        self._mapper = mapper
+
     @property
     def cfgRoot(self):
-        if self._cfgRoot is not None:
-            return self._cfgRoot
-        else:
-            return self.root
+        return self._cfgRoot if self._cfgRoot is not None else self.root
+
+    @property
+    def root(self):
+        return self._root if self._root is not None else self._cfgRoot
 
     @staticmethod
     def inputRepo(storage, tags=None):
@@ -78,15 +89,19 @@ class Repository(object):
     """Represents a repository of persisted data and has methods to access that data.
     """
 
-    def __init__(self, repositoryCfg):
-        '''Initialize a Repository with parameters input via config.
+    def __init__(self, repoData):
+        """Initialize a Repository with parameters input via RepoData.
 
-        :param args: an instance of RepositoryArgs
-        :return:
-        '''
-        self._storage = Storage.makeFromURI(repositoryCfg.root)
-        self._mapperArgs = repositoryCfg.mapperArgs  # keep for reference in matchesArgs
-        self._initMapper(repositoryCfg)
+        Parameters
+        ----------
+        repoData : RepoData
+            Object that contains the parameters with which to init the Repository.
+        """
+        self._storage = Storage.makeFromURI(repoData.cfg.root)
+        if repoData.isNewRepository and not repoData.isV1Repository:
+            self._storage.putRepositoryCfg(repoData.cfg, repoData.args.cfgRoot)
+        self._mapperArgs = repoData.cfg.mapperArgs  # keep for reference in matchesArgs
+        self._initMapper(repoData.cfg)
 
     def _initMapper(self, repositoryCfg):
         '''Initialize and keep the mapper in a member var.
@@ -131,7 +146,7 @@ def _initMapper(self, repositoryCfg):
 
         def __repr__(self):
             return 'config(id=%s, storage=%s, parent=%s, mapper=%s, mapperArgs=%s, cls=%s)' % \
-                   (self.id, self._storage, self.parent, self.mapper, self.mapperArgs, self.cls)
+                   (self.id, self._storage, self.parent, self._mapper, self.mapperArgs, self.cls)
 
     # todo want a way to make a repository read-only
     def write(self, butlerLocation, obj):
@@ -179,7 +194,7 @@ def map(self, *args, **kwargs):
         :return: The type of item is dependent on the mapper being used but is typically a ButlerLocation.
         """
         if self._mapper is None:
-            return None
+            raise RuntimeError("No mapper assigned to Repository")
         loc = self._mapper.map(*args, **kwargs)
         if loc is None:
             return None

python/lsst/daf/persistence/repositoryCfg.py

@@ -34,13 +34,12 @@
 class RepositoryCfg(yaml.YAMLObject):
     yaml_tag = u"!RepositoryCfg_v1"
 
-    def __init__(self, root, mapper, mapperArgs, parents, policy, isLegacyRepository=False):
+    def __init__(self, root, mapper, mapperArgs, parents, policy):
         self._root = root
         self._mapper = mapper
         self._mapperArgs = mapperArgs
         self._parents = iterify(parents)
         self._policy = policy
-        self._isLegacyRepository = isLegacyRepository
 
     @staticmethod
     def v1Constructor(loader, node):
@@ -62,8 +61,7 @@ def v1Constructor(loader, node):
         """
         d = loader.construct_mapping(node)
         cfg = RepositoryCfg(root=d['_root'], mapper=d['_mapper'], mapperArgs=d['_mapperArgs'],
-                            parents=d['_parents'], isLegacyRepository=d['_isLegacyRepository'],
-                            policy=d.get('_policy', None))
+                            parents=d['_parents'], policy=d.get('_policy', None))
         return cfg
 
     def __eq__(self, other):
@@ -72,8 +70,7 @@ def __eq__(self, other):
         return self._root == other._root and \
             self.mapper == other._mapper and \
             self.mapperArgs == other._mapperArgs and \
-            self.parents == other._parents and \
-            self._isLegacyRepository == other._isLegacyRepository
+            self.parents == other._parents
 
     def __ne__(self, other):
         return not self.__eq__(other)
@@ -92,6 +89,12 @@ def root(self, root):
     def mapper(self):
         return self._mapper
 
+    @mapper.setter
+    def mapper(self, mapper):
+        if self._mapper != None:
+            raise RuntimeError("Should not set mapper over previous not-None value.")
+        self._mapper = mapper
+
     @property
     def mapperArgs(self):
         return self._mapperArgs
@@ -110,10 +113,6 @@ def addParents(self, newParents):
             if newParent not in self._parents:
                 self._parents.append(newParent)
 
-    @property
-    def isLegacyRepository(self):
-        return self._isLegacyRepository
-
     @property
     def policy(self):
         return self._policy
@@ -124,7 +123,6 @@ def makeFromArgs(repositoryArgs, parents):
                             mapper=repositoryArgs.mapper,
                             mapperArgs=repositoryArgs.mapperArgs,
                             parents=parents,
-                            isLegacyRepository=repositoryArgs.isLegacyRepository,
                             policy=repositoryArgs.policy)
         return cfg
 
@@ -154,13 +152,12 @@ def matchesArgs(self, repositoryArgs):
         return True
 
     def __repr__(self):
-        return "%s(root=%r, mapper=%r, mapperArgs=%r, parents=%s, policy=%s, isLegacyRepository=%s)" % (
+        return "%s(root=%r, mapper=%r, mapperArgs=%r, parents=%s, policy=%s)" % (
             self.__class__.__name__,
             self._root,
             self._mapper,
             self._mapperArgs,
             self._parents,
-            self._policy,
-            self._isLegacyRepository)
+            self._policy)
 
 yaml.add_constructor(u"!RepositoryCfg_v1", RepositoryCfg.v1Constructor)

python/lsst/daf/persistence/storage.py

@@ -132,3 +132,30 @@ def makeFromURI(uri):
         else:
             raise RuntimeError("No storage registered for scheme %s" % parseRes.scheme)
         return ret
+
+    @staticmethod
+    def isPosix(uri):
+        """Test if a URI is for a local filesystem storage.
+
+        This is mostly for backward compatibility; Butler V1 repositories were only ever on the local
+        filesystem. They may exist but not have a RepositoryCfg class. This enables conditional checking for a
+        V1 Repository.
+
+        This function treats 'file' and '' (no scheme) as posix storages, see
+        the class docstring for more details.
+
+        Parameters
+        ----------
+        uri : string
+            URI to the root of a Repository.
+
+        Returns
+        -------
+        Bool
+            True if the URI is associated with a posix storage, else false.
+        """
+        parseRes = urllib.parse.urlparse(uri)
+        if parseRes.scheme in ('file', ''):
+            return True
+        return False
+

tests/butlerSubset.py

@@ -115,9 +115,9 @@ def registerAliases(butler):
         return butler
 
     def testSingleIteration(self):
-        args = dafPersist.RepositoryArgs(mode='r', root=os.path.join(ROOT, 'butlerSubset'),
+        args = dafPersist.RepositoryArgs(mode='rw', root=os.path.join(ROOT, 'butlerSubset'),
                                          mapper=ImgMapper())
-        butler = dafPersist.Butler(inputs=args)
+        butler = dafPersist.Butler(outputs=args)
 
         ButlerSubsetTestCase.registerAliases(butler)
 

tests/repository.py

@@ -666,7 +666,7 @@ def test(self):
         os.rename(os.path.join(ROOT, 'TestMovedRepositoryCfg/a/repositoryCfg.yaml'),
                   os.path.join(ROOT, 'TestMovedRepositoryCfg/b/repositoryCfg.yaml'))
         butler = dp.Butler(inputs=os.path.join(ROOT, 'TestMovedRepositoryCfg/b'))
-        self.assertEqual(butler._repos.all()[0].cfg,
+        self.assertEqual(list(butler._repos.all().values())[0].cfg,
                          dp.RepositoryCfg(root=os.path.join(ROOT, 'TestMovedRepositoryCfg/b'),
                                           mapper=MapperForTestWriting,
                                           mapperArgs=None,
@@ -742,7 +742,7 @@ def test(self):
                                                                          'TestOutputAlreadyHasParent/a')],
                                                    policy=None))
 
-        # load 'b' as 'read only' and make sure 'a' does not get used as an input.
+        # load 'b' as 'write only' and make sure 'a' does not get used as an input.
         butler = dp.Butler(outputs=os.path.join(ROOT, 'TestOutputAlreadyHasParent/b'))
         self.assertEqual(len(butler._repos.inputs()), 0)
         self.assertEqual(len(butler._repos.outputs()), 1)