-
Notifications
You must be signed in to change notification settings - Fork 12
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
DM-44488: support code for error-handling in ctrl_mpexec #438
Conversation
It should be the responsibility of higher-level catching code to decide whether to log loudly or re-raise instead.
We need this to raise with chaining in the mocks, since that's how it should always be used in production.
6c6b4b4
to
80c412e
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #438 +/- ##
==========================================
- Coverage 81.23% 81.17% -0.07%
==========================================
Files 96 96
Lines 10922 10931 +9
Branches 2085 2086 +1
==========================================
Hits 8873 8873
- Misses 1691 1699 +8
- Partials 358 359 +1 ☔ View full report in Codecov by Sentry. |
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.
I presume the codecov warnings are mitigated because this code is for the tests in ci_middleware?
@@ -202,7 +202,7 @@ def runQuantum(self, butlerQC, inputRefs, outputRefs): | |||
continue | |||
item.metadata.set_dict("failure", failure_info) # type: ignore | |||
|
|||
log.exception( | |||
log.debug( |
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.
Are we sure that we'll still get appropriate outputs in both DRP and AP with this change? I think in AP the full exception will appear, but where will it appear in DRP?
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.
Yes, in DRP SingleQuantumExecutor
will emit that "considering this a qualified success" message, following by another warning with the exc_info
argument that prints the traceback.
Right. The code in |
Checklist
doc/changes
(in ctrl_mpexec)