Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-22364: Allow makeRepo returned Config to be used to create Butler #208

Merged
merged 5 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions python/lsst/daf/butler/core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ def __init__(self, other=None):

if isinstance(other, Config):
self._data = copy.deepcopy(other._data)
self.configFile = other.configFile
elif isinstance(other, collections.abc.Mapping):
self.update(other)
elif isinstance(other, str):
Expand Down Expand Up @@ -814,9 +815,14 @@ def dumpToFile(self, path):
----------
path : `str`
Path to the file to use for output.

Notes
-----
The name of the config file is stored in the Config object.
"""
with open(path, "w") as f:
self.dump(f)
self.configFile = path

def dumpToS3File(self, bucket, key):
"""Writes the config to a file in S3 Bucket.
Expand Down
17 changes: 14 additions & 3 deletions python/lsst/daf/butler/core/repoRelocation.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,27 @@ def replaceRoot(configRoot, butlerRoot):
Butler root directory. Absolute path is inserted into the
``configRoot`` where the
`~lsst.daf.butler.core.repoRelocation.BUTLER_ROOT_TAG` string is
found. If `None` the current working directory is used for the root.
found.

Returns
-------
newRoot : `str`
New configuration string, with the root tag replaced with the butler
root if that tag was present in the supplied configuration root.

Raises
------
ValueError
Raised if ``butlerRoot`` is not set but a value is required.
"""

if butlerRoot is None:
butlerRoot = os.path.curdir
# Do nothing if there is nothing to be done
if BUTLER_ROOT_TAG not in configRoot:
return configRoot

# None or empty string indicate a problem
if not butlerRoot:
raise ValueError(f"Required to replace {BUTLER_ROOT_TAG} in '{configRoot}' "
"but a replacement has not been defined")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you think of any way a common high-level operation would get to this error? If so, we may want to beef up the error message with a guess about what might have gone wrong, but I'm guessing the other changes on this ticket mean it's hard to imagine how this might happen, and that's fine too.


return configRoot.replace(BUTLER_ROOT_TAG, os.path.abspath(butlerRoot))
16 changes: 11 additions & 5 deletions tests/test_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,13 @@ def testMakeRepo(self):
# Remove the file created in setUp
os.unlink(self.tmpConfigFile)

Butler.makeRepo(self.root, config=Config(self.configFile))
butlerConfig = Butler.makeRepo(self.root, config=Config(self.configFile))
limited = Config(self.configFile)
butler1 = Butler(self.root, collection="ingest")
Butler.makeRepo(self.root, standalone=True, createRegistry=False,
config=Config(self.configFile))
butler1 = Butler(butlerConfig, collection="ingest")
butlerConfig = Butler.makeRepo(self.root, standalone=True, createRegistry=False,
config=Config(self.configFile))
full = Config(self.tmpConfigFile)
butler2 = Butler(self.root, collection="ingest")
butler2 = Butler(butlerConfig, collection="ingest")
# Butlers should have the same configuration regardless of whether
# defaults were expanded.
self.assertEqual(butler1.config, butler2.config)
Expand All @@ -410,6 +410,12 @@ def testMakeRepo(self):
self.assertEqual(collections1, set())
self.assertEqual(butler2.registry.getAllCollections(), collections1)

# Check that a config with no associated file name will not
# work properly with relocatable Butler repo
butlerConfig.configFile = None
with self.assertRaises(ValueError):
Butler(butlerConfig, collection="ingest")

def testStringification(self):
butler = Butler(self.tmpConfigFile)
butlerStr = str(butler)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_sqlRegistry.py
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ def testInsertConflictSqlite(self):
from lsst.daf.butler.registries.sqliteRegistry import InsertOnConflict
from sqlalchemy.dialects import sqlite
except ImportError:
self.skipTest("Test skiped, cannot find imports")
self.skipTest("Test skipped, cannot find imports for sqlite conflict testing")

table = self.makeTestTable()

Expand All @@ -747,7 +747,7 @@ def testInsertConflictPG(self):
from lsst.daf.butler.registries.postgresqlRegistry import PostgreSqlRegistry
from sqlalchemy.dialects import postgresql
except ImportError:
self.skipTest("Test skiped, cannot find imports")
self.skipTest("Test skipped, cannot find postgres imports")

table = self.makeTestTable()

Expand All @@ -773,7 +773,7 @@ def testInsertConflictOracle(self):
try:
from sqlalchemy.dialects import oracle
except ImportError:
self.skipTest("Test skiped, cannot find imports")
self.skipTest("Test skiped, cannot find Oracle imports")

table = self.makeTestTable()

Expand Down