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

Update hcp-scada-provider to fix diamond dependency problem with go-msgpack #15185

Merged
merged 8 commits into from Nov 7, 2022

Conversation

kisunji
Copy link
Contributor

@kisunji kisunji commented Oct 28, 2022

No description provided.

@github-actions github-actions bot added the pr/dependencies PR specifically updates dependencies of project label Oct 28, 2022
@@ -19,11 +19,11 @@ require (
github.com/coredns/coredns v1.6.6
github.com/coreos/go-oidc v2.1.0+incompatible
github.com/docker/go-connections v0.3.0
github.com/envoyproxy/go-control-plane v0.10.1
github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels unexpected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@erichaberkorn do you know if this will be okay? A bunch of these are side effects from updating scada provider

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It theoretically should be fine 😂 I'm more concerned about the protobuf upgrade.

go.mod Show resolved Hide resolved
@kisunji kisunji added the pr/no-changelog PR does not need a corresponding .changelog entry label Oct 28, 2022
@@ -132,8 +133,6 @@ func (c *ClientConnPool) dial(datacenter string, serverType string) (*grpc.Clien
grpc.WithContextDialer(c.dialer),
grpc.WithDisableRetry(),
grpc.WithStatsHandler(agentmiddleware.NewStatsHandler(metrics.Default(), metricsLabels)),
// nolint:staticcheck // there is no other supported alternative to WithBalancerName
grpc.WithBalancerName("pick_first"),
Copy link
Contributor Author

@kisunji kisunji Oct 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is gone from the API but "pick_first" was the default balancer anyways (source) so this should have no impact 🤞

@@ -132,8 +133,6 @@ func (c *ClientConnPool) dial(datacenter string, serverType string) (*grpc.Clien
grpc.WithContextDialer(c.dialer),
grpc.WithDisableRetry(),
grpc.WithStatsHandler(agentmiddleware.NewStatsHandler(metrics.Default(), metricsLabels)),
// nolint:staticcheck // there is no other supported alternative to WithBalancerName
grpc.WithBalancerName("pick_first"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are you going to turn on the "pick the first server every time" behavior now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe "pick_first" is the default balancer.

If I'm wrong, then this should be the new way to define balancer config: weaveworks/common#239 (comment)

@rboyer
Copy link
Member

rboyer commented Oct 28, 2022

Does this need a 1.14 backport?

@kisunji kisunji added the backport-inactive/1.14 This release series is longer active label Oct 28, 2022
@kisunji kisunji requested review from rboyer and a team October 28, 2022 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-inactive/1.14 This release series is longer active pr/dependencies PR specifically updates dependencies of project pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants