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

feat: support GEP-1731 #5024

Merged
merged 5 commits into from
Jan 14, 2025
Merged

feat: support GEP-1731 #5024

merged 5 commits into from
Jan 14, 2025

Conversation

zirain
Copy link
Member

@zirain zirain commented Jan 8, 2025

fixes: #3993

@zirain zirain requested a review from a team as a code owner January 8, 2025 10:34
@codecov Codecov
Copy link

codecov bot commented Jan 8, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 84.31373% with 8 lines in your changes missing coverage. Please review.

Project coverage is 66.70%. Comparing base (271a697) to head (0ad8cbb).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/ir/xds.go 11.11% 8 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #5024      +/-   ##
==========================================
- Coverage   66.73%   66.70%   -0.03%     
==========================================
  Files         210      210              
  Lines       32609    32647      +38     
==========================================
+ Hits        21761    21778      +17     
- Misses       9537     9554      +17     
- Partials     1311     1315       +4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

arkodg
arkodg reviewed Jan 8, 2025
View reviewed changes
internal/xds/translator/route.go
@@ -573,8 +572,8 @@ func buildHashPolicy(httpRoute *ir.HTTPRoute) []*routev3.RouteAction_HashPolicy
}

func buildRetryPolicy(route *ir.HTTPRoute) (*routev3.RetryPolicy, error) {
rr := route.Traffic.Retry
any, err := protocov.ToAnyWithValidation(&previoushost.PreviousHostsPredicate{})
rr := route.GetRetry()
Copy link
Contributor

Choose a reason for hiding this comment

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

looks clean !
cc @guydc

arkodg
arkodg reviewed Jan 8, 2025
View reviewed changes
release-notes/current.yaml Outdated Show resolved Hide resolved
arkodg
arkodg reviewed Jan 8, 2025
View reviewed changes
internal/ir/xds.go Outdated Show resolved Hide resolved
arkodg
arkodg reviewed Jan 8, 2025
View reviewed changes
internal/gatewayapi/route.go Outdated Show resolved Hide resolved
arkodg
arkodg reviewed Jan 8, 2025
View reviewed changes
internal/gatewayapi/route.go Outdated Show resolved Hide resolved
@arkodg
Copy link
Contributor

arkodg commented Jan 8, 2025

thanks @zirain , this PR looks good, added some minor comments !

@zirain zirain requested review from arkodg and guydc January 9, 2025 01:03
@zirain zirain force-pushed the gep-1731 branch 3 times, most recently from 8209d49 to 2556f64 Compare January 11, 2025 00:52
arkodg
arkodg previously approved these changes Jan 11, 2025
View reviewed changes
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg requested review from a team January 11, 2025 01:25
@zirain zirain requested a review from arkodg January 13, 2025 02:21
arkodg
arkodg previously approved these changes Jan 13, 2025
View reviewed changes
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

@arkodg arkodg requested review from a team January 13, 2025 18:56
zirain added 5 commits January 14, 2025 06:30
@zirain
feat: support GEP-1731
6e5408e 
Signed-off-by: zirain <zirain2009@gmail.com>
@zirain
release-notes
0e5f4bf 
Signed-off-by: zirain <zirain2009@gmail.com>
@zirain
lint
965e562 
Signed-off-by: zirain <zirain2009@gmail.com>
@zirain
address comment
2c63f61 
Signed-off-by: zirain <zirain2009@gmail.com>
@zirain
comment
0ad8cbb 
Signed-off-by: zirain <zirain2009@gmail.com>
@zirain zirain requested a review from arkodg January 13, 2025 22:35
@arkodg arkodg requested review from a team January 13, 2025 22:38
@arkodg arkodg merged commit 58a9b23 into envoyproxy:main Jan 14, 2025
25 checks passed
@zirain zirain deleted the gep-1731 branch January 14, 2025 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arkodg arkodg arkodg approved these changes

@zhaohuabing zhaohuabing zhaohuabing approved these changes

@guydc guydc Awaiting requested review from guydc

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

support GEP-1731 Configurable Retries
3 participants
@zirain @arkodg @zhaohuabing