Skip to content

Remove strict equality for error message#632

Merged
mum4k merged 2 commits intoenvoyproxy:mainfrom
nareddyt:error-message-fix
Feb 23, 2021
Merged

Remove strict equality for error message#632
mum4k merged 2 commits intoenvoyproxy:mainfrom
nareddyt:error-message-fix

Conversation

@nareddyt
Copy link
Copy Markdown
Contributor

Recommend not to check for strict equality on error messages. This is aligned with Google Cloud APIs error model.

We are considering adding extra debug information to the underlying JSON parser library. The extra debug information breaks this test if it checks for strict equality.

Signed-off-by: Teju Nareddy nareddyt@google.com

Signed-off-by: Teju Nareddy <nareddyt@google.com>
Signed-off-by: Teju Nareddy <nareddyt@google.com>
Copy link
Copy Markdown
Collaborator

@mum4k mum4k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the improvement.

@mum4k mum4k added the waiting-for-review A PR waiting for a review. label Feb 22, 2021
@mum4k
Copy link
Copy Markdown
Collaborator

mum4k commented Feb 23, 2021

@nareddyt #633 fixes the docker CI failure. Would you be able to merge from main to allow the CI to pass?

@mum4k mum4k added waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. and removed waiting-for-review A PR waiting for a review. labels Feb 23, 2021
@mum4k
Copy link
Copy Markdown
Collaborator

mum4k commented Feb 23, 2021

Actually the merge wasn't necessary, the docker test is now passing. The remaining failure is likely a flake, retrying.

@mum4k mum4k added waiting-for-review A PR waiting for a review. and removed waiting-for-changes A PR waiting for comments to be resolved and changes to be applied. labels Feb 23, 2021
@mum4k mum4k merged commit 9b374f6 into envoyproxy:main Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-review A PR waiting for a review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants