-
Notifications
You must be signed in to change notification settings - Fork 138
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
PMM-12702 Fix SSLSni checker. #2625
Conversation
managed/models/agentversion.go
Outdated
func (e *AgentNotSupportedError) Is(err error) bool { | ||
_, ok := err.(*AgentNotSupportedError) | ||
return ok | ||
} |
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.
Why it's needed? It's analog of errors.Is()
, no?
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.
errors.Is didn't work correctly and actually calls this method to compare, so I had to implement it this way.
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.
https://github.com/percona/pmm/pull/2625/files#diff-8792e067d067e501754c905b86a46d9101c2861f211cc265cdc5fd395a2e4bc4R81 here is where errors.Is is called
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.
It seems that we need As
there instead of Is
. We need to check error type, but not exact copy.
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.
As didn't work as well
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.
Why? What's so special about this case?
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 have no idea
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.
the reason was because IsAgentSupported
was returning a pointer and that's why it didn't work.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2625 +/- ##
==========================================
+ Coverage 42.73% 42.76% +0.03%
==========================================
Files 391 391
Lines 49241 49241
==========================================
+ Hits 21043 21058 +15
+ Misses 26218 26203 -15
Partials 1980 1980
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
managed/models/agentversion.go
Outdated
@@ -38,17 +38,22 @@ func (e *AgentNotSupportedError) Error() string { | |||
e.AgentID, e.AgentVersion, e.MinAgentVersion) | |||
} | |||
|
|||
func (e *AgentNotSupportedError) Is(err error) bool { | |||
_, ok := err.(*AgentNotSupportedError) |
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.
🚫 [golangci] reported by reviewdog 🐶
type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)
PMM-12702
Link to the Feature Build: SUBMODULES-3449
If this PR adds or removes or alters one or more API endpoints, please review and add or update the relevant API documents as well:
If this PR is related to some other PRs in this or other repositories, please provide links to those PRs: