Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

PMM-9377 add compatibility with updated pmm #323

Closed
wants to merge 5 commits into from

Conversation

@BupycHuk BupycHuk self-requested a review March 13, 2022 12:23
@idoqo idoqo force-pushed the PMM-9377-update-grpc-gateway branch from 919d316 to 5ec6530 Compare March 14, 2022 09:55
Comment on lines -88 to +90
if errors.Is(err, e) {
if strings.Contains(err.Error(), e.Error()) {
Copy link
Member

Choose a reason for hiding this comment

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

please add a comment why it's changed

Copy link
Contributor Author

@idoqo idoqo Mar 14, 2022

Choose a reason for hiding this comment

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

Some errors contain variable data (e.g connection closed errors include the server IP and port), and those changes between environments so I think it makes sense to check for the relevant substring, as the complete error is not likely to be the same.

you think it should be a code comment?

Copy link
Member

Choose a reason for hiding this comment

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

yeah, let's make it a code comment

client/channel/channel_test.go Outdated Show resolved Hide resolved
@JiriCtvrtka
Copy link
Contributor

LGTM, please add comment where Nurlan wanted it.

@idoqo idoqo force-pushed the PMM-9377-update-grpc-gateway branch 5 times, most recently from a9545df to d5d98b6 Compare March 21, 2022 07:57
@idoqo idoqo force-pushed the PMM-9377-update-grpc-gateway branch from d5d98b6 to f476e71 Compare March 28, 2022 08:09
@idoqo idoqo closed this May 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants