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
Exit to the shell with the number of failed dataRefs #38
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great improvement! Just a few nitpicks below.
python/lsst/pipe/base/cmdLineTask.py
Outdated
@@ -369,12 +373,16 @@ def __call__(self, args): | |||
self.log.MDC("LABEL", str([ref.dataId for ref in dataRef if hasattr(ref, "dataId")])) | |||
task = self.makeTask(args=args) | |||
result = None # in case the task fails | |||
exitStatus = 0 # exit status for the shell |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Picky: code standards say two spaces between the functional end of the line and the comment.
if self.doRaise: | ||
result = task.run(dataRef, **kwargs) | ||
else: | ||
try: | ||
result = task.run(dataRef, **kwargs) | ||
except Exception as e: | ||
exitStatus = 1 # n.b. The shell exit value is the number of dataRefs returning | ||
# non-zero, so the actual value used here is lost |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what code standards say about trailing multiline comments, but I'm gonna guess they want two spaces between the end of the functional part and the start of the comment.
python/lsst/pipe/base/cmdLineTask.py
Outdated
try: | ||
nFailed = sum(((res.exitStatus != 0) for res in resultList)) | ||
except Exception as e: | ||
parsedCmd.log.warn("Unable to retrieve exit status (%s); assuming success" % e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not important in this context, but in general you shouldn't use %
to expand the string but separate with commas:
parsedCmd.log.warn("Unable to retrieve exit status (%s); assuming success", e)
(Another below).
@@ -347,6 +347,8 @@ def __init__(self, name, usage="%(prog)s input [options]", **kwargs): | |||
self.add_argument("--debug", action="store_true", help="enable debugging output?") | |||
self.add_argument("--doraise", action="store_true", | |||
help="raise an exception on error (else log a message and continue)?") | |||
self.add_argument("--noExit", action="store_true", | |||
help="Do not exit even upon failure (i.e. return a struct to the bin script)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe don't assume that the caller's a "bin script"?
Can be overridden using --noExit
a09b5fd
to
2903ab2
Compare
Can be overridden using --noExit