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

grpc: delete deprecated API WithBalancerName() #5232

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Mar 9, 2022

Summary of changes:

  • Deleted the WithBalancerName() API which has been deprecated for ages now.
    • This simplifies code in the ClientConn since we no longer need to handle special cases around balancer name being specified through a dial option
  • Test cleanups
    • Use WithDefaultServiceConfig instead of WithBalancerName wherever applicable
    • Use WithTransportCredentials(insecure.NewCredentials()) instead of WithInsecure() in tests already touched by the above change
    • Use defaultTestTimeout wherever applicable
    • Turn setupClient and setupServer functions into test helpers and significantly reduce boiler plate in individual tests in healthcheck_test.go

Fixes #5229

RELEASE NOTES:

@easwars easwars requested a review from dfawley March 9, 2022 17:38
@easwars easwars added the Type: API Change Breaking API changes (experimental APIs only!) label Mar 9, 2022
@easwars easwars added this to the 1.46 Release milestone Mar 9, 2022
@johanbrandhorst
Copy link
Contributor

Hi! I'm here from the new release notes. I think there is some confusion going on, because the release notes (and this PR) claims that WithBalancerName was deprecated 4 years ago, in #1697. However, it appears it was introduced 4 years ago. It appears to have been deprecated in #2900, which was 3 years ago. Could you perhaps provide a migration guide for users of this API to the release notes?

@menghanl
Copy link
Contributor

menghanl commented Apr 22, 2022

For use cases that set the balancer for the client, grpc.WithDefaultServiceConfig should work.

- grpc.WithBalancerName(balancerName)
+ grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"loadBalancingConfig": [{"%s":{}}]}`, balancerName))

Note that there can be problems if the client receives service configs later with different load balancing policy (or without load balancing policy). The newer service config will override the default service config.
This behavior is different from grpc.WithBalancerName, where the load balancing policy in service config will be ignored.
There's also grpc.WithDisableServiceConfig, if you don't want service config at all.

apesternikov added a commit to apesternikov/core that referenced this pull request Jun 9, 2022
psFried pushed a commit to gazette/core that referenced this pull request Jun 15, 2022
* upgrade grpc to v1.47.0 & address deleted WithBalancerName that was deprecated for 2 years grpc/grpc-go#5232

* restore go.mod and go.sum
michaelschiff pushed a commit to michaelschiff/core that referenced this pull request Jun 16, 2022
* upgrade grpc to v1.47.0 & address deleted WithBalancerName that was deprecated for 2 years grpc/grpc-go#5232

* restore go.mod and go.sum
michaelschiff pushed a commit to michaelschiff/core that referenced this pull request Jun 16, 2022
* upgrade grpc to v1.47.0 & address deleted WithBalancerName that was deprecated for 2 years grpc/grpc-go#5232

* restore go.mod and go.sum
aarongable added a commit to letsencrypt/boulder that referenced this pull request Sep 1, 2022
Changelog: grpc/grpc-go@v1.36.1...v1.49.0

The biggest change for us is that grpc.WithBalancerName has
transitioned from deprecated to fully removed. The fix is to replace
it with a JSON-formatted "default config" object, as demonstrated in
grpc/grpc-go#5232 (comment).

This should unblock updating other dependencies which want to
transitively update gRPC as well.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: API Change Breaking API changes (experimental APIs only!)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

grpc: delete deprecated dial option WithBalancerName
4 participants