-
Notifications
You must be signed in to change notification settings - Fork 4
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-49095: MPGraphExecutor: fix failure to fail #326
Conversation
Not enough arguments for format string.
35af13c
to
84e2948
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## main #326 +/- ##
=======================================
Coverage 90.84% 90.84%
=======================================
Files 50 50
Lines 4686 4686
Branches 409 409
=======================================
Hits 4257 4257
Misses 308 308
Partials 121 121 ☔ View full report in Codecov by Sentry. |
@@ -557,7 +557,7 @@ def _executeQuantaInProcess(self, graph: QuantumGraph, report: Report) -> None: | |||
fail_exit_code = exc.EXIT_CODE | |||
raise | |||
except InvalidQuantumError as exc: | |||
_LOG.fatal("Invalid quantum error for %s (%s): %s", task_node.label, qnode.quantum.dataId) | |||
_LOG.fatal("Invalid quantum error for %s (%s):", task_node.label, qnode.quantum.dataId) |
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.
Actually, isn't it better to combine the two _LOG calls here?
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.
Oh, yeah, probably for the log aggregators; I haven't learned to keep that use case in my head when thinking about logging.
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 think there is a second copy of this _LOG.fatal
with incorrect number of args couple of hundred lines above.
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.
Unfortunately @PaulPrice merged before he saw this discussion.
Not enough arguments for format string.
Checklist
doc/changes