Skip to content

Commit

Permalink
Use new-style configs inside ap_verify.
Browse files Browse the repository at this point in the history
This change removes ap_verify's dependency on the deprecated makeApdb
function. Although the new configs allow better support for Cassandra
APDBs, ap_verify itself only supports SQL (particularly SQLite), given
its restricted scope.
  • Loading branch information
kfindeisen committed Apr 27, 2024
1 parent ffe20f2 commit fea709f
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 38 deletions.
42 changes: 27 additions & 15 deletions python/lsst/ap/verify/pipeline_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand All @@ -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 "
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
----------
Expand All @@ -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

Expand All @@ -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):
Expand Down Expand Up @@ -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)
36 changes: 13 additions & 23 deletions tests/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -93,29 +93,24 @@ 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):
"""Test that runApPipeGen3 places a database in the workspace location by default.
"""
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):
Expand All @@ -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
Expand Down

0 comments on commit fea709f

Please sign in to comment.