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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-20570: Pipeline failure treated as ap_verify success #69

Merged
merged 3 commits into from
Aug 8, 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
4 changes: 3 additions & 1 deletion bin.src/ap_verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#

import sys
from lsst.ap.verify import runApVerify

if __name__ == "__main__":
runApVerify()
result = runApVerify()
sys.exit(result)
4 changes: 3 additions & 1 deletion bin.src/ingest_dataset.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#

import sys
from lsst.ap.verify import runIngestion

if __name__ == "__main__":
runIngestion()
result = runIngestion()
sys.exit(result)
4 changes: 2 additions & 2 deletions doc/lsst.ap.verify/command-line-reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ These two arguments are mandatory, all others are optional.
Status code
===========

:command:`ap_verify.py` returns a status code of ``0`` if the pipeline ran to completion.
If the pipeline fails, the status code will be an interpreter-dependent nonzero value.
Like :ref:`command-line tasks <command-line-task-argument-reference>`, :command:`ap_verify.py` returns the number of data IDs that could not be processed (i.e., 0 on a complete success).
However, an uncaught exception causes :command:`ap_verify.py` to return an interpreter-dependent nonzero value instead (also as for command-line tasks).

.. _ap-verify-cmd-args:

Expand Down
36 changes: 34 additions & 2 deletions python/lsst/ap/verify/ap_verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,12 @@ def runApVerify(cmdLine=None):
cmdLine : `list` of `str`
an optional command line used to execute `runApVerify` from other
Python code. If `None`, `sys.argv` will be used.

Returns
-------
nFailed : `int`
The number of data IDs that were not successfully processed, up to 127,
or 127 if the task runner framework failed.
"""
lsst.log.configure()
log = lsst.log.Log.getLogger('ap.verify.ap_verify.main')
Expand All @@ -152,8 +158,34 @@ def runApVerify(cmdLine=None):
ingestDataset(args.dataset, workspace)

log.info('Running pipeline...')
expandedDataIds = runApPipe(workspace, args)
computeMetrics(workspace, expandedDataIds, args)
apPipeResults = runApPipe(workspace, args)
computeMetrics(workspace, apPipeResults.parsedCmd.id, args)

return _getCmdLineExitStatus(apPipeResults.resultList)


def _getCmdLineExitStatus(resultList):
"""Return the exit status following the conventions of
:ref:`running a CmdLineTask from the command line
<command-line-task-argument-reference>`.

Parameters
----------
resultList : `list` [`Struct`] or `None`
A list of `Struct`, as returned by `ApPipeTask.parseAndRun`. Each
element must contain at least an ``exitStatus`` member.

Returns
-------
exitStatus : `int`
The number of failed runs in ``resultList``, up to 127, or 127 if
``resultList`` is `None`.
"""
if resultList:
# ApPipeTaskRunner does not override default results handling, exitStatus always defined
return min(127, sum(((res.exitStatus != 0) for res in resultList)))
else:
return 127


def runIngestion(cmdLine=None):
Expand Down
7 changes: 4 additions & 3 deletions python/lsst/ap/verify/pipeline_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,9 @@ def runApPipe(workspace, parsedCmdLine):

Returns
-------
dataIds : `lsst.pipe.base.DataIdContainer`
The set of complete data IDs fed into ``ap_pipe``.
apPipeReturn : `Struct`
The `Struct` returned from `~lsst.ap.pipe.ApPipeTask.parseAndRun` with
``doReturnResults=False``.
kfindeisen marked this conversation as resolved.
Show resolved Hide resolved
"""
log = lsst.log.Log.getLogger('ap.verify.pipeline_driver.runApPipe')

Expand All @@ -89,7 +90,7 @@ def runApPipe(workspace, parsedCmdLine):
results = apPipe.ApPipeTask.parseAndRun(pipelineArgs)
log.info('Pipeline complete')

return results.parsedCmd.id
return results


def _getConfigArguments(workspace):
Expand Down
5 changes: 3 additions & 2 deletions tests/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def wrapper(self, *args, **kwargs):
argumentParser=None,
parsedCmd=parsedCmd,
taskRunner=None,
resultList=[None])
resultList=[Struct(exitStatus=0)])
dbPatcher = unittest.mock.patch("lsst.ap.verify.pipeline_driver.makePpdb")
pipePatcher = unittest.mock.patch("lsst.ap.pipe.ApPipeTask",
**{"parseAndRun.return_value": parReturn},
Expand Down Expand Up @@ -123,7 +123,8 @@ def testRunApPipeSteps(self, mockDb, mockClass):
def testRunApPipeDataIdReporting(self, _mockDb, _mockClass):
"""Test that runApPipe reports the data IDs that were processed.
"""
ids = pipeline_driver.runApPipe(self.workspace, self.apPipeArgs)
results = pipeline_driver.runApPipe(self.workspace, self.apPipeArgs)
ids = results.parsedCmd.id

self.assertEqual(ids.idList, _getDataIds())

Expand Down