diff --git a/python/lsst/ap/verify/pipeline_driver.py b/python/lsst/ap/verify/pipeline_driver.py index 2925a5de..ba3aaaba 100644 --- a/python/lsst/ap/verify/pipeline_driver.py +++ b/python/lsst/ap/verify/pipeline_driver.py @@ -35,9 +35,9 @@ import subprocess import logging +import lsst.dax.apdb as daxApdb import lsst.ctrl.mpexec.execFixupDataId # not part of lsst.ctrl.mpexec import lsst.ctrl.mpexec.cli.pipetask -from lsst.ap.pipe.make_apdb import makeApdb _LOG = logging.getLogger(__name__) @@ -60,7 +60,7 @@ def __init__(self): help="A custom version of the ap_verify pipeline (e.g., with different metrics). " "Defaults to the ApVerify.yaml within --dataset.") self.add_argument("--db", "--db_url", default=None, - help="A location for the AP database, formatted as if for ApdbConfig.db_url. " + help="A location for the AP database, formatted as if for apdb-cli create-sql. " "Defaults to an SQLite file in the --output directory.") self.add_argument("--skip-pipeline", action="store_true", help="Do not run the AP pipeline itself. This argument is useful " @@ -91,7 +91,7 @@ def runApPipeGen3(workspace, parsedCmdLine, processes=1): """ log = _LOG.getChild('runApPipeGen3') - makeApdb(_getApdbArguments(workspace, parsedCmdLine)) + _makeApdb(workspace, _getApdbArguments(workspace, parsedCmdLine)) pipelineFile = _getPipelineFile(workspace, parsedCmdLine) pipelineArgs = ["pipetask", "--long-log", "run", @@ -197,8 +197,8 @@ def _getPipelineFile(workspace, parsed): def _getApdbArguments(workspace, parsed): - """Return the config options for running make_apdb.py on this workspace, - as command-line arguments. + """Return the arguments for running apdb-cli create-sql on this workspace, + as key-value pairs. Parameters ---------- @@ -210,14 +210,14 @@ def _getApdbArguments(workspace, parsed): Returns ------- - args : `list` of `str` - Command-line arguments calling ``--config`` or ``--config-file``, - following the conventions of `sys.argv`. + args : mapping [`str`] + Arguments to `lsst.dax.apdb.sql.Apdb.init_database`. """ if not parsed.db: parsed.db = "sqlite:///" + workspace.dbLocation - args = ["--config", "db_url=" + parsed.db] + args = {"db_url": parsed.db, + } return args @@ -239,14 +239,12 @@ def _getConfigArgumentsGen3(workspace, parsed): Command-line arguments calling ``--config`` or ``--config-file``, following the conventions of `sys.argv`. """ - # Translate APDB-only arguments to work as a sub-config - args = [("diaPipe:apdb." + arg if arg != "--config" else arg) - for arg in _getApdbArguments(workspace, parsed)] - args.extend([ + return [ + # APDB config should have been stored in the workspace. + "--config", "parameters:apdb_config=" + workspace.dbConfigLocation, # Put output alerts into the workspace. "--config", "diaPipe:alertPackager.alertWriteLocation=" + workspace.alertLocation, - ]) - return args + ] def _getCollectionArguments(workspace, reuse): @@ -284,3 +282,17 @@ def _getCollectionArguments(workspace, reuse): if reuse and oldRuns: args.extend(["--extend-run", "--skip-existing"]) return args + + +def _makeApdb(workspace, args): + """Create an APDB and store its config for future use. + + Parameters + ---------- + workspace : `lsst.ap.verify.workspace.Workspace` + A Workspace in which to store the database config. + args : mapping [`str`] + Arguments to `lsst.dax.apdb.sql.Apdb.init_database`. + """ + config = daxApdb.ApdbSql.init_database(**args) + config.save(workspace.dbConfigLocation) diff --git a/tests/test_driver.py b/tests/test_driver.py index 04fc6a24..758b89a9 100644 --- a/tests/test_driver.py +++ b/tests/test_driver.py @@ -46,7 +46,7 @@ def patchApPipeGen3(method): """ @functools.wraps(method) def wrapper(self, *args, **kwargs): - dbPatcher = unittest.mock.patch("lsst.ap.verify.pipeline_driver.makeApdb") + dbPatcher = unittest.mock.patch("lsst.ap.verify.pipeline_driver._makeApdb") patchedMethod = dbPatcher(method) return patchedMethod(self, *args, **kwargs) return wrapper @@ -93,13 +93,13 @@ def testrunApPipeGen3Steps(self): self.assertTrue(self.workspace.analysisButler.exists("apdb_marker", id)) self.assertTrue(self.workspace.analysisButler.exists("goodSeeingDiff_assocDiaSrc", id)) - def _getCmdLineArgs(self, parseAndRunArgs): - if parseAndRunArgs[0]: - return parseAndRunArgs[0][0] - elif "args" in parseAndRunArgs[1]: - return parseAndRunArgs[1]["args"] + def _getArgs(self, call_args): + if call_args.args: + return call_args.args[1] + elif "args" in call_args.kwargs: + return call_args.kwargs["args"] else: - self.fail("No command-line args passed to parseAndRun!") + self.fail(f"No APDB args passed to {call_args}!") @patchApPipeGen3 def testrunApPipeGen3WorkspaceDb(self, mockDb): @@ -107,15 +107,10 @@ def testrunApPipeGen3WorkspaceDb(self, mockDb): """ pipeline_driver.runApPipeGen3(self.workspace, self.apPipeArgs) - # Test the call to make_apdb.py mockDb.assert_called_once() - cmdLineArgs = self._getCmdLineArgs(mockDb.call_args) - self.assertIn("db_url=sqlite:///" + self.workspace.dbLocation, cmdLineArgs) - - # Test the call to the AP pipeline - id = _getDataIds(self.workspace.analysisButler)[0] - apdbConfig = self.workspace.analysisButler.get("apdb_marker", id) - self.assertEqual(apdbConfig.db_url, "sqlite:///" + self.workspace.dbLocation) + dbArgs = self._getArgs(mockDb.call_args) + self.assertIn("db_url", dbArgs) + self.assertEqual(dbArgs["db_url"], "sqlite:///" + self.workspace.dbLocation) @patchApPipeGen3 def testrunApPipeGen3WorkspaceCustom(self, mockDb): @@ -124,15 +119,10 @@ def testrunApPipeGen3WorkspaceCustom(self, mockDb): self.apPipeArgs.db = "postgresql://somebody@pgdb.misc.org/custom_db" pipeline_driver.runApPipeGen3(self.workspace, self.apPipeArgs) - # Test the call to make_apdb.py mockDb.assert_called_once() - cmdLineArgs = self._getCmdLineArgs(mockDb.call_args) - self.assertIn("db_url=" + self.apPipeArgs.db, cmdLineArgs) - - # Test the call to the AP pipeline - id = _getDataIds(self.workspace.analysisButler)[0] - apdbConfig = self.workspace.analysisButler.get("apdb_marker", id) - self.assertEqual(apdbConfig.db_url, self.apPipeArgs.db) + dbArgs = self._getArgs(mockDb.call_args) + self.assertIn("db_url", dbArgs) + self.assertEqual(dbArgs["db_url"], self.apPipeArgs.db) def testrunApPipeGen3Reuse(self): """Test that runApPipeGen3 does not run the pipeline at all (not even with