-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Print the correct underlying cause for OCI errors #7968
Print the correct underlying cause for OCI errors #7968
Conversation
@mheon @vrothberg PTAL |
@xordspar0 Any chance you could whip up a test to make sure the error message is formatted correctly going forward? |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan, xordspar0 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
LGTM |
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.
LGTM
A test would be good. I guess a unit tests would be sufficient. |
Sure. For a unit test I'll need to break the logic out to a separate function that can take arguments. Maybe that's better anyway. |
Previously, the order of OCI error messages was reversed, so that the type of error was listed as the cause. For example: Error: writing file `cpu.cfs_quota_us`: Invalid argument: OCI runtime error This error message makes it seem like "OCI runtime error" is the argument that was invalid. In fact, "OCI runtime error" is the error and "writing file ..." is the cause. With this change, the above message reads: Error: OCI runtime error: writing file `cpu.cfs_quota_us`: Invalid argument Signed-off-by: Jordan Christiansen <[email protected]>
The tests are all passing. This should be good to merge. There's something wrong with Cirrus that makes it keep losing its connection with the agent nodes, but other than that everything is green. |
/lgtm |
🎉 Thanks for the thoughtful reviews and suggestions, everyone! |
Previously, the order of OCI error messages was reversed, so that the
type of error was listed as the cause. For example:
This error message makes it seem like "OCI runtime error" is the
argument that was invalid. In fact, "OCI runtime error" is the error and
"writing file ..." is the cause. With this change, the above message
reads:
Signed-off-by: Jordan Christiansen [email protected]