add tri-state verification to mongodb detector #1575
Conversation
| defer func() { | ||
| if err := client.Disconnect(ctx); err != nil { | ||
| return | ||
| } | ||
| }() |
There was a problem hiding this comment.
@ahrav i think this was just discarding the result of client.Disconnect so i simplified it a bit when i moved it, but give a shout if the new version looks wrong
| defer func() { | ||
| if err := client.Disconnect(ctx); err != nil { | ||
| return | ||
| } | ||
| }() |
| } | ||
| results = append(results, s1) | ||
| } | ||
|
|
||
| return results, nil | ||
| } | ||
|
|
||
| func isErrDeterminate(err error) bool { |
There was a problem hiding this comment.
suggestion: I think you can simplify this with:
if e, ok := err.(topology.ConnectionError); ok {
_, ok = e.Unwrap().(*auth.Error)
return ok
}
return false
or i guess if we want to use the errors pkg:
func isErrDeterminate(err error) bool {
var connErr topology.ConnectionError
if errors.As(err, &connErr) {
var authErr *auth.Error
return errors.As(connErr.Unwrap(), &authErr)
}
return false
}
There was a problem hiding this comment.
i actually specifically wrote this not to be as terse as possible, but to make it easy to extend with other error cases in the future. do you find it hard to read as it is?
There was a problem hiding this comment.
ahh okay, i think that makes sense. I sorta had the feeling, but wasn't sure. If we leave the switches might be worth adding the default case to each, no mandatory though. One of the the go linters happens to yell at me when i have a switch with no default. 🤷
| if err != nil { | ||
| return err | ||
| } | ||
| defer func() { |
There was a problem hiding this comment.
nit: since we are ignoring the err here anyways we can inline it:
defer client.Disconnect(ctx)
There was a problem hiding this comment.
huh, i could have sworn the linter didn't let me do that...
There was a problem hiding this comment.
might've complained that you are ignoring the err i suppose.
There was a problem hiding this comment.
yeah golangci-lint is not about this
This reverts commit 2e3b1d3.
No description provided.