job-exec: fix confusing "job shell exec error" log message #6579
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
grondo
force-pushed
the
issue#6568
branch
2 times, most recently
from
7033da9 to
9d1af0e
Compare
|
Apparently there was a test that explicitly checked that the job shell path was emitted in the exec error. Removed that test and repushed. |
Problem: A test in t2400-job-exec-test.t checks that the path to the job is emitted on a job-exec subprocess exec error, but it has been determined that including this path in error message is more confusing than helpful. Remove the test.
Problem: When the job-exec module detects an exec error for a job shell it emits a confusing error message that includes either the path to the job shell or the IMP (if a multiuser job), and only the result of `strerror()` for the errno returned from libsubprocess. When using sdexec, this errno is always `ENOENT`, resulting in a confusing error message that seems to indicate that `flux-imp` was not found. It is unhelpful to include `argv[0]` in this error message. It will always be the job shell or the IMP and we all know it. Drop this from the log message. Also, sdexec will provide extra information in the subprocess error string available from `flux_subprocess_fail_error (p)`. Log this instead of `strerror (errno)`. Fixes flux-framework#6568
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6579 +/- ##
==========================================
- Coverage 79.47% 79.46% -0.01%
==========================================
Files 531 531
Lines 88421 88421
==========================================
- Hits 70274 70267 -7
- Misses 18147 18154 +7
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem: When the job-exec module detects an exec error for a job shell it emits a confusing error message that includes either the path to the job shell or the IMP (if a multiuser job), and only the result of
strerror()for the errno returned from libsubprocess. When using sdexec, this errno is alwaysENOENT, resulting in a confusing error message that seems to indicate thatflux-impwas not found.It is unhelpful to include
argv[0]in this error message. It will always be the job shell or the IMP and we all know it. Drop this from the log message.Also, sdexec will provide extra information in the subprocess error string available from
flux_subprocess_fail_error (p). Log this instead ofstrerror (errno).Fixes #6568