Skip to content

resolver: create AddressMapV2 with generics to replace AddressMap #8187

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
Merged
merged 4 commits into from
Mar 21, 2025
Merged

resolver: create AddressMapV2 with generics to replace AddressMap #8187

merged 4 commits into from
Mar 21, 2025

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented Mar 20, 2025 •
edited
Loading

RELEASE NOTES:

  • resolver: Add experimental AddressMapV2 with generics to ultimately replace AddressMap. Deprecate AddressMap for deletion.

@dfawley dfawley added the Type: API Change Breaking API changes (experimental APIs only!) label Mar 20, 2025
@dfawley dfawley requested a review from arjan-bal March 20, 2025 20:47
@codecov Codecov
Copy link

codecov bot commented Mar 20, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 94.59459% with 2 lines in your changes missing coverage. Please review.

Project coverage is 81.97%. Comparing base (d8924ac) to head (e3ea5e5).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
balancer/pickfirst/pickfirstleaf/pickfirstleaf.go 94.11% 1 Missing ⚠️
resolver/map.go 92.30% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8187      +/-   ##
==========================================
- Coverage   81.99%   81.97%   -0.03%     
==========================================
  Files         410      410              
  Lines       40237    40233       -4     
==========================================
- Hits        32993    32981      -12     
- Misses       5873     5884      +11     
+ Partials     1371     1368       -3
Files with missing lines Coverage Δ
balancer/base/balancer.go 48.29% <100.00%> (-0.70%) ⬇️
balancer/weightedroundrobin/balancer.go 83.42% <100.00%> (ø)
balancer/pickfirst/pickfirstleaf/pickfirstleaf.go 86.60% <94.11%> (-0.11%) ⬇️
resolver/map.go 94.39% <92.30%> (-1.80%) ⬇️

... and 28 files with indirect coverage changes

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

@arjan-bal arjan-bal added this to the 1.72 Release milestone Mar 20, 2025
@arjan-bal
Copy link
Contributor

I don't have any reason to believe this is used widely (if at all) outside our project, and it's experimental, and it's very easy to migrate to, so I believe it's fine to just swap in place without a migration period. Let me know if you feel strongly otherwise.

RELEASE NOTES:

  • resolver: convert experimental AddressMap to use generics

Discussed offline and we found usages in SourceGraph. The PR now keeps the old AddressMap and deprecates it.

@arjan-bal arjan-bal assigned dfawley and unassigned arjan-bal Mar 21, 2025
@dfawley dfawley changed the title resolver: convert AddressMap to use generics resolver: create AddressMapV2 with generics to replace AddressMap Mar 21, 2025
@dfawley dfawley merged commit b0d1203 into grpc:master Mar 21, 2025
15 checks passed
@dfawley dfawley deleted the AddressMapGenerics branch March 21, 2025 20:10
@dfawley dfawley mentioned this pull request Mar 21, 2025
Merged
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Mar 24, 2025
@dfawley @janardhankrishna-sai
resolver: create AddressMapV2 with generics to replace AddressMap (gr…
328c513 
…pc#8187)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arjan-bal arjan-bal arjan-bal approved these changes

Assignees

@dfawley dfawley

Labels
Type: API Change Breaking API changes (experimental APIs only!)
Projects
None yet
Milestone
1.72 Release
Development

Successfully merging this pull request may close these issues.
2 participants
@dfawley @arjan-bal