feat(translator): implement timeout in ClientTrafficPolicy#2667
feat(translator): implement timeout in ClientTrafficPolicy#2667arkodg merged 10 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Yael Shechter <yael.shechter@sap.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2667 +/- ##
==========================================
+ Coverage 63.38% 63.41% +0.02%
==========================================
Files 119 119
Lines 19098 19156 +58
==========================================
+ Hits 12106 12147 +41
- Misses 6193 6212 +19
+ Partials 799 797 -2 ☔ View full report in Codecov by Sentry. |
| return err | ||
| } | ||
|
|
||
| if httpIR.Timeout == nil { |
There was a problem hiding this comment.
Small nit, but maybe this is slightly more readable as a switch statement:
switch {
case httpIR.Timeout == nil:
httpIR.Timeout = &ir.ClientTimeout{}
fallthrough
case httpIR.Timeout.HTTP == nil:
httpIR.Timeout.HTTP = &ir.HTTPClientTimeout{}
}
httpIR.Timeout.HTTP.RequestReceivedTimeout = &metav1.Duration{
Duration: d,
}There was a problem hiding this comment.
Thanks, Lior
I think that this suggestion offers a cleaner structure, but is it a convention for implementing switch cases? I'm not sure that all readers are familiar with the fallthrough keyword.
There was a problem hiding this comment.
The fallthrough keyword is part of the language - readers should be familiar with the language.
But this can also be written without it:
switch {
case httpIR.Timeout == nil:
httpIR.Timeout = &ir.ClientTimeout{
HTTP: &ir.HTTPClientTimeout{},
}
case httpIR.Timeout.HTTP == nil:
httpIR.Timeout.HTTP = &ir.HTTPClientTimeout{}
}
httpIR.Timeout.HTTP.RequestReceivedTimeout = &metav1.Duration{
Duration: d,
}Signed-off-by: Yael Shechter <yael.shechter@sap.com>
Signed-off-by: Yael Shechter <yael.shechter@sap.com>
Signed-off-by: Yael Shechter <yael.shechter@sap.com>
Signed-off-by: Yael Shechter <yael.shechter@sap.com>
Signed-off-by: Yael Shechter <yael.shechter@sap.com>
Signed-off-by: Yael Shechter <yael.shechter@sap.com>
Signed-off-by: Yael Shechter <yael.shechter@sap.com>
|
/retest |
zirain
left a comment
There was a problem hiding this comment.
LGTM, looking for doc and e2e with following PRs.
What type of PR is this?
Implement Timeout in ClientTrafficPolicy
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #2598