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 to use linkedin/go-zk #820

Merged
merged 2 commits into from
Jan 25, 2025
Merged

Update to use linkedin/go-zk #820

merged 2 commits into from
Jan 25, 2025

Conversation

trvrnrth
Copy link
Contributor

The primary driver behind this is to be able to make use of the new default host provider added in linkedin/go-zk#6 to resolve connection issues when zookeeper host IPs change.

@trvrnrth trvrnrth requested a review from bai as a code owner June 13, 2024 15:33
@@ -36,12 +35,7 @@ type BurrowZookeeperClient struct {
// timeout it's possible to reestablish a connection to a different server and keep the same session. This is means any
// ephemeral nodes and watches are maintained.
func ZookeeperConnect(servers []string, sessionTimeout time.Duration, logger *zap.Logger) (protocol.ZookeeperClient, <-chan zk.Event, error) {
// We need a function to set the logger for the ZK connection
Copy link
Contributor Author

Choose a reason for hiding this comment

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

go-zk now uses slog

@bbc88ks
Copy link

bbc88ks commented Jan 24, 2025

Is there a reason why this has not been reviewed/merged yet? I think we are bumping into the same thing.

@bai bai merged commit 42c68c7 into linkedin:master Jan 25, 2025
1 check passed
github.com/spf13/viper v1.18.2
github.com/stretchr/testify v1.9.0
github.com/prometheus/client_golang v1.20.5
github.com/samuel/go-zookeeper v0.0.0-20201211165307-7117e9ea2414
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This package should no longer be required.

@trvrnrth
Copy link
Contributor Author

@bai Thanks for getting this included. It just needs a go mod tidy following your merge of master as the old package is still being required.

@bai
Copy link
Collaborator

bai commented Jan 25, 2025

Thanks for your contribution! Will fix that in #837 in a moment.

@trvrnrth trvrnrth deleted the go-zk branch January 25, 2025 12:41
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.

3 participants