-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
fix (kubernetes-client-api) : Remove opinionated messages from Config's errorMessages
and deprecate it (#5166)
#5199
Conversation
a960e84
to
3d9a624
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.
If the logic to update the error message is removed, then let's just remove the errorMessages from the Config rather than deprecating them - if you use a ConfigBuilder (and don't read the changelog) you won't know that it's a deprecated field.
I hope to get around to sundrio/sundrio#383 that could help us have deprecated builder methods eventually.
3d9a624
to
879954b
Compare
…'s `errorMessages` and deprecate it (fabric8io#5166) + Remove opinionated status code to message mappings from Config + Deprecate Config.errorMessages field in favor of kubernetes status + Update OperationSupport to only rely on Kubernetes status for error messages Signed-off-by: Rohan Kumar <[email protected]>
879954b
to
ef47584
Compare
SonarCloud Quality Gate failed. |
assertThatExceptionOfType(KubernetesClientException.class) | ||
.isThrownBy(eventResource::delete) | ||
.withMessageContaining(customMessage) | ||
.hasFieldOrPropertyWithValue("code", HTTP_FORBIDDEN); |
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'm going to approve and merge this anyway, but please (once more) note:
Don't use constants in the test assertions, especially if those constants are also used when defining the test expectations. This is an external constant and there's no problem, but if we had defined this constant internally wrongly with a 503
instead, the test would be passing and not revealing a problem
This field was deprecated in fabric8io#5199 , we should remove this in v7 Signed-off-by: Rohan Kumar <[email protected]>
This field was deprecated in #5199 , we should remove this in v7 Signed-off-by: Rohan Kumar <[email protected]>
Description
Fix #5166
Type of change
test, version modification, documentation, etc.)
Checklist