-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
remove max drain duration #50508
remove max drain duration #50508
Conversation
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
@@ -1390,10 +1390,6 @@ func ValidateDrainDuration(drainTime *durationpb.Duration) (errs error) { | |||
errors.New("drain time only supports durations to seconds precision")) | |||
} | |||
|
|||
if drainDuration > drainTimeMax { |
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.
Why it was there?
If remove, drainTimeMax should be removed too
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.
Do not the know the history. But seems not needed. Removed the var also
Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
/test integ-security |
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
I wouldn't recommend setting it to 100hr personally... but if someone wants to 🤷
Same here |
Some protocols needs more drain duration it seems #50477. Envoy does not have max - So not sure if we should have one