Skip to content

clientv3: fix init client error#14368

Merged
ahrtr merged 1 commit intoetcd-io:mainfrom
happlins:main
Nov 6, 2022
Merged

clientv3: fix init client error#14368
ahrtr merged 1 commit intoetcd-io:mainfrom
happlins:main

Conversation

@happlins
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

I have modified and added unit tests.

@SimFG
Copy link
Copy Markdown
Contributor

SimFG commented Aug 23, 2022

@happlins you need to combine four commits into one

@happlins
Copy link
Copy Markdown
Contributor Author

sorry,I have revised it.

Comment thread client/v3/client_test.go Outdated
@happlins happlins changed the title clentv3: fix init client error clientv3: fix init client error Aug 30, 2022
@ahrtr
Copy link
Copy Markdown
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
Copy Markdown
Member

ahrtr commented Oct 13, 2022

any update on this?

@happlins
Copy link
Copy Markdown
Contributor Author

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

Comment thread client/v3/client_test.go Outdated
Comment thread client/v3/client_test.go Outdated
@happlins happlins force-pushed the main branch 2 times, most recently from ab68bfe to d3eea7c Compare October 13, 2022 08:59
Comment thread client/v3/client_test.go Outdated
Signed-off-by: happlins <happlins@foxmail.com>
Copy link
Copy Markdown
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
Copy Markdown
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.

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