-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
xds: add xDS transport custom Dialer support #7586
xds: add xDS transport custom Dialer support #7586
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7586 +/- ##
==========================================
+ Coverage 81.74% 81.87% +0.12%
==========================================
Files 361 361
Lines 27813 27827 +14
==========================================
+ Hits 22736 22783 +47
+ Misses 3868 3848 -20
+ Partials 1209 1196 -13
|
99a59fc
to
354e088
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test that works at an even higher level? I.e. registers the creds builder from outside of the xdsclient package and confirms that a grpc client created using grpc.NewClient
will invoke the proper dialer?
EDIT: finished comments; oops!
(Sorry, hit send too soon; updated comment above.) |
Done. The test setup is inspired by other tests under |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM aside from the two small comments -- thanks!
+@easwars for a second review |
…ent for transport test
Design:
Dialer
method that specifies how to dial the xDS server via the credentials bundle without introducing new public APIs.Tested:
./scripts/vet.sh
go test -cpu 1,4 -timeout 7m ./...
go test -race -cpu 1,4 -timeout 7m ./...
RELEASE NOTES: