-
Notifications
You must be signed in to change notification settings - Fork 561
WIP: 🌱 remove no longer required grpc replace (issue: #3284) #3529
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
base: master
Are you sure you want to change the base?
WIP: 🌱 remove no longer required grpc replace (issue: #3284) #3529
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
d24dbb4
to
b7965e6
Compare
b7965e6
to
bf220e4
Compare
Motivation: operator-framework/operator-lifecycle-manager#3529 (comment) Upstream-repository: operator-lifecycle-manager Upstream-commit: 5d3239d70edbdfa051c41db517698d355cb5c4a7
Motivation: operator-framework/operator-lifecycle-manager#3529 (comment) Upstream-repository: operator-lifecycle-manager Upstream-commit: 5d3239d70edbdfa051c41db517698d355cb5c4a7
Motivation: operator-framework/operator-lifecycle-manager#3529 (comment) Upstream-repository: operator-lifecycle-manager Upstream-commit: 5d3239d70edbdfa051c41db517698d355cb5c4a7
bf220e4
to
af5502b
Compare
af5502b
to
87be2e2
Compare
87be2e2
to
fdc4da1
Compare
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
@@ -163,7 +163,7 @@ func grpcConnection(address string) (*grpc.ClientConn, error) { | |||
})) | |||
} | |||
|
|||
return grpc.Dial(address, dialOptions...) | |||
return grpc.NewClient(address, dialOptions...) |
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 not sure we should make this change. I see that grpc.Dial
is deprecated, but reading the GoDoc of grpc.DialContext
(which is called by grpc.Dial
), it sounds like there are subtle differences that may matter to us:
// One subtle difference between NewClient and Dial and DialContext is that the
// former uses "dns" as the default name resolver, while the latter use
// "passthrough" for backward compatibility. This distinction should not matter
// to most users, but could matter to legacy users that specify a custom dialer
// and expect it to receive the target string directly.
We do use a custom dialer if there is a proxyURL.
Motivation: operator-framework/operator-lifecycle-manager#3529 (comment) Upstream-repository: operator-lifecycle-manager Upstream-commit: 5d3239d70edbdfa051c41db517698d355cb5c4a7
Closes: #3284