-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[Network] Add AppGateway subresource tests #4137
Changes from 1 commit
a9f306d
e281f88
02a9e4e
65e0f2c
38a8b5c
e8d2626
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -432,7 +432,7 @@ def update_ag_backend_http_settings_collection(instance, parent, item_name, port | |
instance.request_timeout = timeout | ||
if connection_draining_timeout is not None: | ||
instance.connection_draining.enabled = bool(connection_draining_timeout) | ||
instance.connection_draining.drain_timeout_in_sec = connection_draining_timeout | ||
instance.connection_draining.drain_timeout_in_sec = connection_draining_timeout or 1 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have to set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. The service will say that it has to be greater than 1 (though ironically it only has to be equal to 1). |
||
if host_name is not None: | ||
instance.host_name = host_name | ||
if host_name_from_backend_pool is not None: | ||
|
Large diffs are not rendered by default.
Large diffs are not rendered by default.
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.
If I got it right, we should do the flip if the values exist, so we need a
else
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.
From the description of
invert
it seems we would only invert the flag behavior (i.e. the flag implies false instead of true). @devigned do you concur?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.
Yes, examining the two cases where this is used, it's clear that we do need to invert the values as well. Thanks for pointing this out!