Skip to content

Commit

Permalink
Fix Database.sync return type to match its docs.
Browse files Browse the repository at this point in the history
  • Loading branch information
TallJimbo committed Mar 4, 2020
1 parent 8be92eb commit 35d718f
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 8 deletions.
3 changes: 2 additions & 1 deletion python/lsst/daf/butler/registry/_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,8 @@ def registerRun(self, name: str):
"""
id = self._runIdsByName.get(name)
if id is None:
(id,), _ = self._db.sync(self._tables.run, keys={"name": name}, returning=["id"])
row, _ = self._db.sync(self._tables.run, keys={"name": name}, returning=["id"])
id = row["id"]
self._runIdsByName[name] = id
self._runNamesById[id] = name
# Assume that if the run is in the cache, it's in the database, because
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/daf/butler/registry/interfaces/_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ def check():
elif bad:
raise DatabaseConflictError(f"Conflict in sync on column(s) {bad}.")
inserted = False
return result, inserted
return {k: v for k, v in zip(returning, result)} if returning is not None else None, inserted

def insert(self, table: sqlalchemy.schema.Table, *rows: dict, returnIds: bool = False,
) -> Optional[List[int]]:
Expand Down
12 changes: 6 additions & 6 deletions python/lsst/daf/butler/registry/tests/_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,32 +322,32 @@ def testSync(self):
# Insert a row with sync, because it doesn't exist yet.
values, inserted = db.sync(tables.b, keys={"name": "b1"}, extra={"value": 10}, returning=["id"])
self.assertTrue(inserted)
self.assertEqual([{"id": values[0], "name": "b1", "value": 10}],
self.assertEqual([{"id": values["id"], "name": "b1", "value": 10}],
[dict(r) for r in db.query(tables.b.select()).fetchall()])
# Repeat that operation, which should do nothing but return the
# requested values.
values, inserted = db.sync(tables.b, keys={"name": "b1"}, extra={"value": 10}, returning=["id"])
self.assertFalse(inserted)
self.assertEqual([{"id": values[0], "name": "b1", "value": 10}],
self.assertEqual([{"id": values["id"], "name": "b1", "value": 10}],
[dict(r) for r in db.query(tables.b.select()).fetchall()])
# Repeat the operation without the 'extra' arg, which should also just
# return the existing row.
values, inserted = db.sync(tables.b, keys={"name": "b1"}, returning=["id"])
self.assertFalse(inserted)
self.assertEqual([{"id": values[0], "name": "b1", "value": 10}],
self.assertEqual([{"id": values["id"], "name": "b1", "value": 10}],
[dict(r) for r in db.query(tables.b.select()).fetchall()])
# Repeat the operation with a different value in 'extra'. That still
# shouldn't be an error, because 'extra' is only used if we really do
# insert. Also drop the 'returning' argument.
_, inserted = db.sync(tables.b, keys={"name": "b1"}, extra={"value": 20})
self.assertFalse(inserted)
self.assertEqual([{"id": values[0], "name": "b1", "value": 10}],
self.assertEqual([{"id": values["id"], "name": "b1", "value": 10}],
[dict(r) for r in db.query(tables.b.select()).fetchall()])
# Repeat the operation with the correct value in 'compared' instead of
# 'extra'.
_, inserted = db.sync(tables.b, keys={"name": "b1"}, compared={"value": 10})
self.assertFalse(inserted)
self.assertEqual([{"id": values[0], "name": "b1", "value": 10}],
self.assertEqual([{"id": values["id"], "name": "b1", "value": 10}],
[dict(r) for r in db.query(tables.b.select()).fetchall()])
# Repeat the operation with an incorrect value in 'compared'; this
# should raise.
Expand All @@ -368,7 +368,7 @@ def testSync(self):
tables = context.addTableTuple(STATIC_TABLE_SPECS)
_, inserted = rodb.sync(tables.b, keys={"name": "b1"})
self.assertFalse(inserted)
self.assertEqual([{"id": values[0], "name": "b1", "value": 10}],
self.assertEqual([{"id": values["id"], "name": "b1", "value": 10}],
[dict(r) for r in rodb.query(tables.b.select()).fetchall()])
with self.assertRaises(ReadOnlyDatabaseError):
rodb.sync(tables.b, keys={"name": "b2"}, extra={"value": 20})
Expand Down

0 comments on commit 35d718f

Please sign in to comment.