Add connection draining support to application gateway by jobatzil · Pull Request #913 · Azure/azure-rest-api-specs

jobatzil · 2017-02-07T01:25:17Z

This PR adds the support of connection draining to application gateways.
Starts with api-version 2016-12-01.

…ion. Added 2016-09-01 and 2016-12-01 folder to project.

…ionGateway.json for version 2016-12-01.

…son to the 2016-12-01 applicationGateway.json version.

… WAF).

azurecla · 2017-02-07T01:25:47Z

Hi @jobatzil, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!

It looks like you're working at Microsoft (jobatzil). If you're full-time, we DON'T require a contribution license agreement.

If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

amarzavery · 2017-02-07T02:30:52Z

@jobatzil - Can you please copy the previous api-version 2016-09-01 and paste it in the "2016-12-01/swagger/applicationGateway.json" file. In the next commit, overwrite the file with new changes. This will help us see the diff and makes reviewing easier.

Update: I noticed different commits.

One more request - Can you please provide examples for request and response using the extension x-ms-examples? This will

help us validate the examples against the swagger spec and catch errors early on and
it will also form the basis for customer facing REST API documentation on docs.microsoft.com website.

You can more info about the extension and an example swagger spec over here.

jobatzil · 2017-02-07T02:35:56Z

@amarzavery
The first commit of this PR contains the old version of the applicationgateway.json file, I just copied it and added it to the project (along with some other files that weren't in the project yet). The second commit then contains the changes!

Okay, I can do that.

amarzavery · 2017-02-07T02:36:34Z

@jobatzil - Yeah, I saw that and updated the comment.

vishrutshah · 2017-02-07T03:44:47Z

@amarzavery / @olydis looks like refs are not getting resolved.

Referenced in Swagger: https://github.com/Azure/azure-rest-api-specs/pull/913/files#diff-6b98d4930c0dc642745ceff99f0efa0cR717

Travis Job: https://travis-ci.org/Azure/azure-rest-api-specs/jobs/199061417#L2312

amarzavery · 2017-02-07T03:51:58Z

arm-network/2016-12-01/swagger/applicationGateway.json

+        "backendIPConfigurations": {
+          "type": "array",
+          "items": {
+            "$ref": "./networkInterface.json#/definitions/NetworkInterfaceIPConfiguration"


which is true. Because there is no networkInterface.json file in the 2016-12-01/swagger folder. It is present in 2016-09-01/swagger folder.

The bigger problem here is:

Can we reference the "NetworkInterfaceIPConfiguration" model from a swagger that is having api-version "2016-09-01"?

If yes then wouldn't it break the theory that "api-version" is a contract. One should return models that represent the schema as per that api-version

If one copies over the "networkInterface.json" file to 2016-12-01 api-version folder then is NRP ready to provision that version of the NetworkInterface API in all the regions in AzureProd ?

If NRP is indeed ready with this api-version for networkInterface (i.e. they have already deployed this api-version) then why have they not sent a PR with an updated swagger spec for the new api-version ("2016-12-01").

jobatzil · 2017-02-07T03:52:23Z

@vishrutshah I've talked to @amarzavery about that and sent an email to Deepak Rajendran.
Not quite sure whether creating a copy of that file in the new Api-Version folder or referencing the file of the old api-version is the way to go here. Or should it be automatically resolved to the most recent version?

vishrutshah · 2017-02-07T23:29:12Z

arm-network/2016-12-01/swagger/applicationGateway.json

+            "modelAsString": true
+          }
+        },
+        "connectionDraining": {


Is connectionDraining optional/required property?

vishrutshah · 2017-02-07T23:30:21Z

arm-network/2016-12-01/swagger/applicationGateway.json

+    },
+    "ApplicationGatewayConnectionDraining": {
+        "properties": {
+            "enabled": {


Is enable required/options?

If optional, what is the default value for enabled?

required. Good to know, thanks!

vishrutshah · 2017-02-07T23:31:18Z

arm-network/2016-12-01/swagger/applicationGateway.json

+                "type": "boolean",
+                "description": "Defines if connection draining is enabled or not."
+            },
+            "drainTimeoutInSec": {


Please leverage minimum: 1 & maximum: 3600 on drainTimeoutInSec property.

done, good to know, thanks!

vishrutshah · 2017-02-07T23:48:19Z

arm-network/2016-12-01/swagger/applicationGateway.json

+  ],
+  "consumes": [
+    "application/json",
+    "text/json"


WARNING: NonAppJsonTypeWarning - Media types other than 'application/json' has limited support

vishrutshah · 2017-02-07T23:48:26Z

arm-network/2016-12-01/swagger/applicationGateway.json

+  ],
+  "produces": [
+    "application/json",
+    "text/json"


WARNING: NonAppJsonTypeWarning - Media types other than 'application/json' has limited support

…ectionDraining in applicationGateway.json

…09-01 networkInterface.json as discussed.

…connection_draining

amarzavery · 2017-02-09T06:53:39Z

arm-network/2016-12-01/swagger/applicationGateway.json

+        "backendIPConfigurations": {
+          "type": "array",
+          "items": {
+            "$ref": "./../../2016-09-01/swagger/networkInterface.json#/definitions/NetworkInterfaceIPConfiguration"


@jobatzil The referenced model "NetworkInterfaceIPConfiguration" has a property named "properties" of type "NetworkInterfaceIPConfigurationPropertiesFormat", which contains a property named "applicationGatewayBackendAddressPools" that references "./applicationGateway.json#/definitions/ApplicationGatewayBackendAddressPool".

The above reference is to the 2016-09-01 version of applicationgateway.json.

Isn't that weird. Shouldn't it be referencing the 2016-12-01 version of applicationgateway.json (I mean circling back to this file)?

/cc @DeepakRajendranMsft, @johanste, @olydis, @vishrutshah

Hey Amar, yah I know. You should have gotten an email about that.

azurecla · 2017-02-09T07:24:08Z

Hi @jobatzil, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!

It looks like you're working at Microsoft (jobatzil). If you're full-time, we DON'T require a contribution license agreement.

If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

…enerated comments in azure sdk.

vishrutshah · 2017-02-14T17:16:44Z

@jobatzil Could you please resolve the merge conflict when you get a chance. Thanks!

…ous changes.

jobatzil · 2017-02-14T18:36:00Z

The last 5 commits after the merge be4bf42 are the steps to address the issue of applicationGateway referencing networkInterface which didn't have a 2016-12-01 version yet. The solution (based on some discussions with @amarzavery) is to copy over networkInterface.json to the new version folder as well as all files referenced by it.)

amarzavery · 2017-02-14T19:26:11Z

arm-network/2016-12-01/swagger/applicationGateway.json

+            "modelAsString": true
+          }
+        },
+        "connectionDraining": {


Adding connectionDraining in the middle, may mess up the signature of the model constructor in the generated code. Please verify this in the generated code. I would suggest adding this new property in the end.

amarzavery · 2017-02-14T19:37:26Z

@DeepakRajendranMsft - Can you please make sure that the files moved to 2016-12-01 api-version are good?
Should the pending files checkDnsAvailability, usage, virtualNetworkGateway, networkWathcer be moved to 2016-12-01 as well?
/cc @irrogozh

DeepakRajendranMsft · 2017-02-14T20:18:44Z

Yes, everything needs to be moved

amarzavery · 2017-02-14T20:55:45Z

@jobatzil - Can you please make sure to move all the swagger files from September api-version to December?

…on the comments on github.

…ettings to the end to avoid issues with the optionality of the property

….com/jobatzil/azure-rest-api-specs into json-mode

…updated compositeNetworkClient

…onnection_draining

…face to point to the 2016-12-01 networkInterface.

…fix of amar.

…s into json-mode

…onnection_draining

… project file.

needs to be merged right now

AutorestCI · 2017-02-15T00:55:54Z

Azure/azure-sdk-for-ruby#690

AutorestCI · 2017-02-15T00:56:04Z

Azure/azure-sdk-for-python#996

AutorestCI · 2017-02-15T00:56:47Z

No modification for NodeJS

* add api version for targeting ppe and dev endpoint * spelling * build fixes * reverting last change * updating readme

jobatzil added 4 commits February 6, 2017 15:22

#ConnectionDraining Created applicationGateway.json file for new vers…

2ee070e

…ion. Added 2016-09-01 and 2016-12-01 folder to project.

#ConnectionDraining Added support for connection draining in applicat…

db15014

…ionGateway.json for version 2016-12-01.

#ConnectionDraining Updated the reference in compositeNetworkClient.j…

00add07

…son to the 2016-12-01 applicationGateway.json version.

#ConnectionDraining rephrased 3 descriptions (connection draining and…

0c28774

… WAF).

azurecla added the cla-not-required label Feb 7, 2017

azuresdkci assigned vishrutshah Feb 7, 2017

#ConnectionDraining Changed version entry in applicationGateway.json.

38c3b90

amarzavery reviewed Feb 7, 2017

View reviewed changes

vishrutshah previously requested changes Feb 8, 2017

View reviewed changes

jobatzil added 3 commits February 8, 2017 13:44

#ConnectionDraining Added maximum, minimum and required field to Conn…

1e1d3d4

…ectionDraining in applicationGateway.json

#ConnectionDraining Fixed reference issue, changed reference to 2016-…

9dff907

…09-01 networkInterface.json as discussed.

Merge remote-tracking branch 'upstream/master' into feature/jobatzil/…

272b833

…connection_draining

olydis mentioned this pull request Feb 9, 2017

Autorest change the input filepath and does not found it Azure/autorest#1778

Closed

amarzavery mentioned this pull request Feb 9, 2017

fix cross referencing models in relative swagger specs Azure/autorest#1805

Merged

amarzavery reviewed Feb 9, 2017

View reviewed changes

amarzavery closed this Feb 9, 2017

amarzavery reopened this Feb 9, 2017

azurecla added the cla-not-required label Feb 9, 2017

amarzavery added the Network label Feb 10, 2017

amarzavery mentioned this pull request Feb 10, 2017

Added network watcher APIs #923

Merged

10 tasks

jobatzil added 2 commits February 11, 2017 18:37

#ConnectionDraining Adjusted comments of connection draining to autog…

2342c12

…enerated comments in azure sdk.

Merge branch 'master' into feature/jobatzil/connection_draining

b226918

#ConnectionDraining updated compositeNetworkClient based on the previ…

6b4e11e

…ous changes.

amarzavery reviewed Feb 14, 2017

View reviewed changes

jobatzil and others added 14 commits February 14, 2017 12:56

#ConnectionDraining copied the rest of the files to 2016-12-01 based …

a5691ba

…on the comments on github.

#ConnectionDraining Moved connectionDraining property in BackendHttpS…

626fe3c

…ettings to the end to avoid issues with the optionality of the property

Merge branch 'feature/jobatzil/connection_draining' of https://github…

6bbf74a

….com/jobatzil/azure-rest-api-specs into json-mode

porting vmssNetworkInterface back to 2016-09-01

90d6b63

#ConnectionDraining Removed 2016-12-01 vmssNetworkInterface.json and …

66fc525

…updated compositeNetworkClient

Merge remote-tracking branch 'amar/json-mode' into feature/jobatzil/c…

8b01d1a

…onnection_draining

#ConnectionDraining changed references of 2016-09-01 vmssNetworkInter…

7c597a7

…face to point to the 2016-12-01 networkInterface.

#ConnectionDraining Undo previous changes to prepare for merging the …

46b1cc7

…fix of amar.

modified vmss for 2016-12-01

1f981e1

Merge branch 'json-mode' of github.com:amarzavery/azure-rest-api-spec…

2f4e2bc

…s into json-mode

Merge remote-tracking branch 'amar/json-mode' into feature/jobatzil/c…

3388d75

…onnection_draining

vmss to 2016-12-01 in composite spec

6f1e185

Merge remote-tracking branch 'amar/json-mode' into feature/jobatzil/c…

5562029

…onnection_draining

#ConnectionDraining added 2016-12-01 vmssNetworkInterface.json to the…

2adb5fa

… project file.

amarzavery approved these changes Feb 15, 2017

View reviewed changes

amarzavery merged commit dcd97d4 into Azure:master Feb 15, 2017

AutorestCI mentioned this pull request Feb 15, 2017

Automatic PR from RestAPI-PR913 Azure/azure-sdk-for-ruby#690

Merged

AutorestCI mentioned this pull request Feb 15, 2017

[Network] Updates Azure/azure-sdk-for-python#996

Closed

jobatzil mentioned this pull request Feb 15, 2017

Feature/jobatzil/connection draining Azure/azure-sdk-for-net#2824

Merged

7 tasks

mccleanp pushed a commit that referenced this pull request Mar 23, 2022

add api version for targeting ppe and dev endpoint (#913)

f6a2b95

* add api version for targeting ppe and dev endpoint * spelling * build fixes * reverting last change * updating readme

Conversation

jobatzil commented Feb 7, 2017 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azurecla commented Feb 7, 2017

Uh oh!

amarzavery commented Feb 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jobatzil commented Feb 7, 2017

Uh oh!

amarzavery commented Feb 7, 2017

Uh oh!

vishrutshah commented Feb 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jobatzil commented Feb 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amarzavery Feb 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azurecla commented Feb 9, 2017

Uh oh!

vishrutshah commented Feb 14, 2017

Uh oh!

jobatzil commented Feb 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amarzavery commented Feb 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DeepakRajendranMsft commented Feb 14, 2017

Uh oh!

amarzavery commented Feb 14, 2017

Uh oh!

AutorestCI commented Feb 15, 2017

Uh oh!

AutorestCI commented Feb 15, 2017

Uh oh!

AutorestCI commented Feb 15, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jobatzil commented Feb 7, 2017 •

edited by azuresdkci

Loading

amarzavery commented Feb 7, 2017 •

edited

Loading

vishrutshah commented Feb 7, 2017 •

edited

Loading

amarzavery Feb 9, 2017 •

edited

Loading

amarzavery commented Feb 14, 2017 •

edited

Loading