-
Notifications
You must be signed in to change notification settings - Fork 109
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
Have ErtStorageExceptions not be unexpected in cli #9407
Conversation
What about a test? |
373d5e1
to
bacbf99
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9407 +/- ##
==========================================
+ Coverage 91.83% 91.90% +0.07%
==========================================
Files 434 434
Lines 26735 26736 +1
==========================================
+ Hits 24551 24571 +20
+ Misses 2184 2165 -19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
||
except PermissionError as error: | ||
logger.exception((f"Unexpected exception in ensemble: \n {error!s}")) | ||
await event_unary_send(event_creator(Id.ENSEMBLE_FAILED)) |
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.
Now when this gets propagated I'd expect that you will check such status in the test.(?)
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 was leftovers from 90c14a9 that was included due to some git wizardry 🧙♂️.
I will add some tests for it in a different PR 🌴 🌞
This commit: * Improves the error message displayed when the dark storage server does not have access to the storage path. * Makes the dark storage server return a response with status code 401 - unauthorized when the `get_ensemble_record` endpoint fails due to `PermissionError`. * Makes the failed message in `LegacyEnsemble._evaluate_inner` omit stacktrace when it failed due to PermissionError, making it shorter and more consise.
bacbf99
to
1b75f60
Compare
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.
Good to go! 🚀
This commits makes ErtStorageExceptions not be output as "unexpected error" in cli.
Issue
Resolves #9223
Approach
This PR improves the way ErtStorageExceptions are handled and displayed in the cli. Prior to this PR, ErtStorageExceptions were "unexpected" in the cli.
How it was before this change:
(Screenshot of new behavior in GUI if applicable)
This is now more similar to how it is output in the gui:
git rebase -i main --exec 'pytest tests/ert/unit_tests -n logical -m "not integration_test"'
)When applicable