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.

Sign up for GitHub

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

Jump to bottom

Implement Update method for EndpointManager #12667

Merged
merged 1 commit into from
Feb 4, 2021
Merged

Implement Update method for EndpointManager #12667

merged 1 commit into from
Feb 4, 2021

Conversation

lemonlinger
Copy link

@lemonlinger lemonlinger commented Feb 4, 2021 •
edited
Loading

This is a part of changes for #12652 including:

  • Add testing for the resolver.
  • Implement Update method for EndpointManager.

@peterbourgon

ptabor
ptabor suggested changes Feb 4, 2021
View reviewed changes
Copy link
Contributor

@ptabor ptabor left a comment

Choose a reason for hiding this comment

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

Thank you. That looks awesome.
Just a few 'feature_requests' in the comments.

client/v3/naming/endpoints/endpoints_impl.go Show resolved Hide resolved
tests/integration/clientv3/naming/resolver_test.go Outdated Show resolved Hide resolved
tests/integration/clientv3/naming/resolver_test.go Outdated Show resolved Hide resolved
tests/integration/grpc_stub_server.go Outdated Show resolved Hide resolved
client/v3/naming/endpoints/endpoints_impl.go Show resolved Hide resolved
@ptabor ptabor changed the title Implment Update method for EndpointManager Implement Update method for EndpointManager Feb 4, 2021
ptabor
ptabor approved these changes Feb 4, 2021
View reviewed changes
Copy link
Contributor

@ptabor ptabor left a comment

Choose a reason for hiding this comment

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

Thank you.

@ptabor
Copy link
Contributor

ptabor commented Feb 4, 2021

PASSES="fmt" ./test.sh

Failed with:

go: updates to go.sum needed, disabled by -mod=readonly
FAIL: 'go_fmt_for_package ./...' checking failed (!=0 return code)

Please run ./scripts/fix.sh

@ptabor
Copy link
Contributor

ptabor commented Feb 4, 2021

Still

go: updates to go.sum needed, disabled by -mod=readonly
must be run from 'go.etcd.io/etcd/v3' module directory

Please check go mod tidy in the main directory if the ./scripts/fix.sh does not help.

@lemonlinger
Copy link
Author

Still

go: updates to go.sum needed, disabled by -mod=readonly
must be run from 'go.etcd.io/etcd/v3' module directory

Please check go mod tidy in the main directory if the ./scripts/fix.sh does not help.

Run go mod tidy but nothing happened.

@ptabor
Copy link
Contributor

ptabor commented Feb 4, 2021

Assuming it passes, please squash the commits.

@ptabor
Copy link
Contributor

ptabor commented Feb 4, 2021

Woohu. Don't worry about the flake. Please squash and I will merge.

endpoints: implement Update method for EndpointManager.
571ed50 
- Add integration test for endpoints and resolver.
@lemonlinger lemonlinger force-pushed the implement-endpoint-manager-and-resolver branch from 5b4b4de to 571ed50 Compare February 4, 2021 15:32
@lemonlinger
Copy link
Author

Woohu. Don't worry about the flake. Please squash and I will merge.

Sorry for the slow response. Squashed.

@ptabor
Copy link
Contributor

ptabor commented Feb 4, 2021

Sorry for the slow response. Squashed.
Your response time is impressive. Thank you.

Merging on green.

@ptabor ptabor merged commit dae29bb into etcd-io:master Feb 4, 2021
@ptabor ptabor mentioned this pull request Feb 4, 2021
Closed
@lemonlinger lemonlinger deleted the implement-endpoint-manager-and-resolver branch February 4, 2021 23:23
@chaochn47 chaochn47 mentioned this pull request Oct 20, 2023
Merged
@ahrtr ahrtr mentioned this pull request Mar 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ptabor ptabor ptabor approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lemonlinger @ptabor