Skip to content
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

pkg/deviceplugin: move to grpc.NewClient() #1748

Merged
merged 1 commit into from
May 28, 2024
Merged

Conversation

mythi
Copy link
Contributor

@mythi mythi commented May 28, 2024

grpc.NewClient(), added in grpc-go v1.63, is the preferred way to create a new ClientConn. In most of our usages, moving away from grpc.Dial*() to it is straightforward.

However, we've also relied on grpc.Dial*()'s behavior to automatically make a new connection to "test" a connection is successful isn't available anymore. Combined with grpc.WithBlock dialoption this usage is considered "especially bad" way to handle a client connection.

The recommended approach to test a server connection is to separately make a connection and watch the connection state to become Ready. This change follows that recommendation.

grpc.NewClient(), added in grpc-go v1.63, is the preferred way to
create a new ClientConn. In most of our usages, moving away from
grpc.Dial*() to it is straightforward.

However, we've also relied on grpc.Dial*()'s behavior to automatically
make a new connection to "test" a connection is successful isn't available
anymore. Combined with grpc.WithBlock dialoption this usage is considered
"especially bad" way to handle a client connection.

The recommended approach to test a server connection is to separately
make a connection and watch the connection state to become Ready. This
change follows that recommendation.

Signed-off-by: Mikko Ylinen <[email protected]>
@tkatila tkatila merged commit 20b7b5a into intel:main May 28, 2024
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants