[Network] Add AppGateway subresource tests #4137
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4137 +/- ##
==========================================
+ Coverage 70.54% 70.87% +0.33%
==========================================
Files 437 437
Lines 28226 28239 +13
Branches 4344 4348 +4
==========================================
+ Hits 19912 20015 +103
+ Misses 7017 6879 -138
- Partials 1297 1345 +48
Continue to review full report at Codecov.
|
|
I have incorporated the changes for |
|
ping @troydai |
| else: | ||
| values = values or negative_label | ||
| if not values: | ||
| values = negative_label if invert else positive_label |
There was a problem hiding this comment.
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.
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.
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!
| @@ -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.
Do we have to set 1 when users used 0 to disable the connection_draining? A bit confusing, that you can use 1 to enable; and after you use 0 to disable, but the next read still shows 1 like things not cleaned up. A service side glitch?
There was a problem hiding this comment.
Yes. The service will say that it has to be greater than 1 (though ironically it only has to be equal to 1).
|
@yugangw-msft I updated the three_state_flag logic. |
haroldrandom
added
cla-not-required
Network
Partially addresses #3941.
Fixes issues where
--no-waitwas not honored for subresource deletes.Fixes issues with three_state_flag when using custom labels.
Fixes issue where connection draining could not be disabled with
http-settings update.This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
Command Guidelines
(see Authoring Command Modules)