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

clientv3: fix init client error #14368

Merged
merged 1 commit into from
Nov 6, 2022
Merged

clientv3: fix init client error #14368

merged 1 commit into from
Nov 6, 2022

Conversation

happlins
Copy link
Contributor

@happlins happlins commented Aug 22, 2022

fix clientv3 init client error

close #14323

Signed-off-by: happlins happlins@foxmail.com

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@ahrtr
Copy link
Member

ahrtr commented Aug 22, 2022

Two comments:

  1. please signoff the commit using command git rebase HEAD~1 --signoff;
  2. Please consider to add a couple of unit tests to verify the changes.

@happlins
Copy link
Contributor Author

I have modified and added unit tests.

@SimFG
Copy link
Contributor

SimFG commented Aug 23, 2022

@happlins you need to combine four commits into one

@happlins
Copy link
Contributor Author

sorry,I have revised it.

client/v3/client_test.go Outdated Show resolved Hide resolved
@happlins happlins changed the title clentv3: fix init client error clientv3: fix init client error Aug 30, 2022
@ahrtr
Copy link
Member

ahrtr commented Sep 5, 2022

The minimum version supported should be 3.4, so probably we should update min at client.go#L510.

Please also consider to enhance the unit test to cover more scenarios,such as,

  1. All endpoint versions are up to date;
  2. All endpoint versions are out of date;
  3. Partially endpoint versions are out of date;

@ahrtr
Copy link
Member

ahrtr commented Oct 13, 2022

any update on this?

@happlins
Copy link
Contributor Author

I'm sorry. I've been too busy recently. I've modified the minimum version and added unit test

client/v3/client_test.go Outdated Show resolved Hide resolved
client/v3/client_test.go Outdated Show resolved Hide resolved
@happlins happlins force-pushed the main branch 2 times, most recently from ab68bfe to d3eea7c Compare October 13, 2022 08:59
client/v3/client_test.go Outdated Show resolved Hide resolved
Signed-off-by: happlins <happlins@foxmail.com>
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you @happlins

@ahrtr
Copy link
Member

ahrtr commented Nov 6, 2022

Since there is no response from other maintainers and it's a safe change to me, so let me merge this PR.

@ahrtr ahrtr merged commit b082094 into etcd-io:main Nov 6, 2022
CaojiamingAlan added a commit to CaojiamingAlan/etcd that referenced this pull request Jul 18, 2023
Signed-off-by: caojiamingalan <alan.c.19971111@gmail.com>
ahrtr added a commit that referenced this pull request Jul 19, 2023
@ahrtr ahrtr mentioned this pull request Oct 27, 2023
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Etcd v3 api: Initialization client error
5 participants