Skip to content

resolver: convert EndpointMap to use generics #8189

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

Merged
merged 2 commits into from
Mar 24, 2025

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented Mar 21, 2025

This is #8187 but for EndpointMap. In this case, I could find zero external usages of this type, since it is so new, so I thought it would be OK to just convert it in place.

RELEASE NOTES:

  • resolver: convert EndpointMap to use generics

@dfawley dfawley added the Type: API Change Breaking API changes (experimental APIs only!) label Mar 21, 2025
@dfawley dfawley added this to the 1.72 Release milestone Mar 21, 2025
@dfawley dfawley requested a review from arjan-bal March 21, 2025 20:55
Copy link

codecov bot commented Mar 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.93%. Comparing base (b0d1203) to head (bb928ec).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8189      +/-   ##
==========================================
- Coverage   82.06%   81.93%   -0.14%     
==========================================
  Files         410      410              
  Lines       40233    40216      -17     
==========================================
- Hits        33018    32949      -69     
- Misses       5854     5892      +38     
- Partials     1361     1375      +14     
Files with missing lines Coverage Δ
balancer/endpointsharding/endpointsharding.go 93.10% <100.00%> (-0.08%) ⬇️
balancer/leastrequest/leastrequest.go 88.49% <100.00%> (-0.21%) ⬇️
balancer/weightedroundrobin/balancer.go 83.23% <100.00%> (-0.19%) ⬇️
resolver/map.go 94.39% <100.00%> (ø)
xds/internal/balancer/outlierdetection/balancer.go 89.36% <100.00%> (+0.95%) ⬆️
xds/internal/balancer/ringhash/ring.go 92.85% <100.00%> (-0.11%) ⬇️
xds/internal/balancer/ringhash/ringhash.go 93.86% <100.00%> (-0.06%) ⬇️

... and 18 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@arjan-bal arjan-bal left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -174,7 +174,7 @@ func (s) TestAddressMap_Values(t *testing.T) {
}

func (s) TestEndpointMap_Length(t *testing.T) {
em := NewEndpointMap()
em := NewEndpointMap[any]()
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use struct{} as a more precise type parameter here, and int in the test cases below. Any reason to use any?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I just did the simplest change possible for the tests, but using the proper type is better.

@arjan-bal arjan-bal assigned dfawley and unassigned arjan-bal Mar 24, 2025
@dfawley dfawley assigned arjan-bal and unassigned dfawley Mar 24, 2025
@dfawley dfawley merged commit a51009d into grpc:master Mar 24, 2025
13 checks passed
@dfawley dfawley deleted the EndpointMapGenerics branch March 24, 2025 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: API Change Breaking API changes (experimental APIs only!)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants