Fix error code returned by cmsRun on a FallbackFileOpenError #42249
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
6 tasks
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42249/36267
|
|
A new Pull Request was created by @wddgit (W. David Dagenhart) for master. It involves the following packages:
@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
wddgit
commented
Jul 12, 2023
Dr15Jones
reviewed
Jul 12, 2023
|
-1 Failed Tests: UnitTests RelVals RelVals-INPUT AddOn Unit TestsI found errors in the following unit tests: ---> test test_cmsLHEtoEOSManager had ERRORS RelValsRelVals-INPUT
Expand to see more relval errors ...AddOn TestsExpand to see more addon errors ... |
wddgit
force-pushed
the
fixErrorReturnCode
branch
from
c41f603 to
40155e0
Compare
cmsbuild
added
tests-pending
code-checks-pending
and removed
tests-rejected
code-checks-approved
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42249/36268
|
|
Pull request #42249 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again. |
|
please test Try tests again. Maybe the failures are some kind of transient glitch unrelated to my code changes. At first glance I thought the test failures might be related to my changes because they had FileOpenError and FallbackFileOpen in the exception messages. But on closer examination, I can't see how my changes could cause all the errors before the exception. Maybe it is even confirmation the changes are working. For example: |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42249/36352
|
|
Pull request #42249 was updated. @cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please check and sign again. |
|
please test Done. I implemented the suggestion from @stlammel. I also rebased. And force pushed it. Should be good to go if the tests pass, but let me know if there is anything else that needs modifying or to be discussed. |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2a7d00/33810/summary.html Comparison SummarySummary:
|
|
@Dr15Jones I think this PR is ready for approval again. I implemented the handling of the exception as Stephan suggested. Let me know if there is anything more I need to do. |
|
+1 |
cmsbuild
added
core-approved
fully-signed
and removed
core-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
This was referenced Jul 26, 2023
7 tasks
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.
PR description:
Fix error code returned by cmsRun on a FallbackFileOpenError. There was a bug where it returns FileOpenError even in the fallback case.
There is more discussion and more information related to this bug in issue #42040. FYI @stlammel
Also added a unit test so this bug will not recur.
Also I noticed a case where a misconfigured storage.json file could cause a seg fault. I added a check for that case and now it throws an exception with an informative error message (remains fatal as before).
PR validation:
New unit test passes and existing unit tests pass. This should only affect the behavior after an exception or seg fault occurs. It should not affect the output of successful cmsRun processes.