Add support for Express Route Provider APIs #2532

gimotwanMSFT · 2018-02-23T01:00:29Z

A new top level NRP resource ExpressRouteCrossConnection is being
introduced with this change. This resource shadows the customer
ExpressRouteCircuit resouce in the customer subscription and enables
the provider to perform CRUD operations on some of the
sub-resources, such as peerings on the ExpressRoute Circuit

* update readme.md to include the complete path for the 2018-02-01
  folder
* add examples
* add ExpressRouteCrossConnection as a top level resource

A new top level NRP resource ExpressRouteCrossConnection is being introduced with this change. This resource shadows the customer ExpressRouteCircuit resouce in the customer subscription and enables the provider to perform CRUD operations on some of the sub-resources, such as peerings on the ExpressRoute Circuit * update readme.md to include the complete path for the 2018-02-01 folder * add examples * add ExpressRouteCrossConnection as a top level resource

AutorestCI · 2018-02-23T01:18:52Z

Automation for azure-sdk-for-python

Nothing to generate for azure-sdk-for-python

AutorestCI · 2018-02-23T01:23:58Z

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

…rk-2018-02-01

dsgouda

Posted a few comments. Please check CI jobs and resolve issues if any

dsgouda · 2018-02-26T19:40:16Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        "tags": [
+          "ExpressRouteCrossConnections"
+        ],
+        "operationId": "ExpressRouteCrossConnections_ListAll",


Recommended operationId for list operations is ExpressRouteCrossConnections_List

dsgouda · 2018-02-26T19:41:35Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        "tags": [
+          "ExpressRouteCrossConnections"
+        ],
+        "operationId": "ExpressRouteCrossConnectionsByResourceGroup_List",


recommended operationId is ExpressRouteCrossConnections_ListByResourceGroup

dsgouda · 2018-02-26T19:45:47Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+          "in": "body",
+          "required": true,
+          "schema": {
+          "$ref": "./network.json#/definitions/TagsObject"


nit: fix indentation
Shouldn't this parameter be named tags instead?

Fixed indentation. The naming is consistent with corresponding operation on ExpressRoute Circuit

Thank you for helping with the reviews.

parameters is still to generic, please name it something more meaningful

dsgouda · 2018-02-26T19:47:17Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        }
+        },
+        "x-ms-examples": {
+        "Update Express Route CrossConnection Tags": {


Remove spaces (basic Json keys' semantics)

dsgouda · 2018-02-26T19:49:47Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        "tags": [
+          "ExpressRouteCrossConnectionArpTable"
+        ],
+        "operationId": "ExpressRouteCrossConnections_ListArpTable",


Rename as ExpressRouteCircuitsArpTable_list

dsgouda · 2018-02-26T19:55:56Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+          "description": "The ExpressRouteCircuit"
+        },
+        "serviceProviderProvisioningState": {
+          "type": "string",


Provisioning states are usually readonly. Please confirm.

The serviceProviderProvisioning state is writeable by the ExpressRoute Provider subscription.

dsgouda · 2018-02-26T19:56:09Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        },
+        "provisioningState": {
+          "type": "string",
+          "description": "Gets the provisioning state of the public IP resource. Possible values are: 'Updating', 'Deleting', and 'Failed'."


Definitely readonly

dsgouda · 2018-02-26T19:56:37Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+          "$ref": "./network.json#/definitions/Resource"
+        }
+      ],
+        "description": "ExpressRouteCrossConnection resource"


nit: indentation

dsgouda · 2018-02-26T19:56:52Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        },
+        "nextLink": {
+          "type": "string",
+          "description": "The URL to get the next set of results."


mark as readonly

Should the nextLink record be marked readOnly ? A quick browse through some of the other files does not show nextLink marked as readOnly. Please advise and I will update.

Yes, please mark this as readonly

dsgouda · 2018-02-26T19:57:33Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+      "in": "path",
+      "required": true,
+      "type": "string",
+    "description": "The subscription credentials which uniquely identify the Microsoft Azure subscription. The subscription ID forms part of the URI for every service call."


nit: indentation

dsgouda

A few more comments.

dsgouda · 2018-02-26T22:53:51Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+      }
+    },
+    "ExpressRouteCrossConnectionServiceProviderProperties": {
+      "properties": {


Similarly, please go through all the models and their properties in the spec and verify their readonly-ness

dsgouda · 2018-02-26T22:54:18Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        },
+        "nextLink": {
+          "type": "string",
+          "description": "The URL to get the next set of results."


Yes, please mark this as readonly

dsgouda · 2018-02-26T22:54:57Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+          "in": "body",
+          "required": true,
+          "schema": {
+          "$ref": "./network.json#/definitions/TagsObject"


parameters is still to generic, please name it something more meaningful

dsgouda · 2018-02-26T22:56:25Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        "tags": [
+        "ExpressRouteCrossConnections"
+        ],
+        "operationId": "ExpressRouteCrossConnections_UpdateTags",


Rename this as ExpressRouteCrossConnectionsTags_Update

dsgouda

A minor comment and please delete the .swp file

dsgouda · 2018-02-26T23:49:01Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+      "properties": {
+        "properties": {
+          "x-ms-client-flatten": true,
+          "$ref": "#/definitions/ExpressRouteCrossConnectionPropertiesFormat"


nit: please rename this as ExpressRouteCrossConnectionProperties

dsgouda · 2018-02-26T23:52:47Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        }
+        },
+        "x-ms-examples": {
+        "UpdateExpressRouteCrossConnectionTags": {


nit: fix indentation

dsgouda · 2018-02-26T23:53:30Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+          "200": {
+            "description": "Request successful. The operation returns the resulting ExpressRouteCrossConnectionsRouteTableSummary resource.",
+            "schema": {
+            "$ref": "./expressRouteCircuit.json#/definitions/ExpressRouteCircuitsRoutesTableSummaryListResult"


nit: Fix indentation

dsgouda · 2018-02-26T23:53:50Z

...etwork/resource-manager/Microsoft.Network/stable/2018-02-01/expressRouteCrossConnection.json

+        ],
+        "responses": {
+          "200": {
+              "description": "Request successful. The operation returns the resulting ExpressRouteCrossConnectionsRouteTable resource.",


Indentation

dsgouda

There 2 more violations you need to fix.

Not all operations have corresponding examples. Please provide them.
Please verify whether information related to the service appears in the List_Operations API for network

dsgouda · 2018-02-27T23:27:35Z

A few examples are still missing, please check here

gimotwanMSFT · 2018-03-01T01:01:42Z

How do I verify the following ?
Please verify whether information related to the service appears in the List_Operations API for network

dsgouda · 2018-03-01T01:03:02Z

@gimotwanMSFT disregard that comment

dsgouda

LGTM

… 2018 (#2642) * Add support for Express Route Circuit Connection resource as a child of Express Route Circuit Peering in 2018-02-01 API (#2358) * copy all files from 2018-01-01 to 2018-02-01 * update api version to 2018 in all files * add express route circuit connection as child of express route bgp peering * update readme.md * add examples * fix travis build error * fix travis build error * fix indentation errors * update reference to circuit connections in peering * fix indentation * Merge Express Route Circuit Connection to Networking2018-02-01 (#2370) * copy all files from 2018-01-01 to 2018-02-01 * update api version to 2018 in all files * add express route circuit connection as child of express route bgp peering * update readme.md * add examples * fix travis build error * fix travis build error * fix indentation errors * update reference to circuit connections in peering * fix indentation * fix indentation * fix indentation' * get latest file * Fix indentation * fix key * fix indentation and example * fix indentation * fix indentation * make circuit con singular * add fix as PR #2364 (#2376) * Network feature: Setting custom ipsec policy for Virtual Network Gateway P2S clients. (#2521) * 1443089: Network feature: Setting custom ipsec policy for Virtual Network Gateway P2S clients. * 1443089:Fix network ReadMe file. * 1443089:Fix network ReadMe file. * Temporary bug fix * Add support for Express Route Provider APIs (#2532) * Add support for Express Route Provider APIs A new top level NRP resource ExpressRouteCrossConnection is being introduced with this change. This resource shadows the customer ExpressRouteCircuit resouce in the customer subscription and enables the provider to perform CRUD operations on some of the sub-resources, such as peerings on the ExpressRoute Circuit * remove wrong enum values from circuit connection status and fix enum … (#2572) * remove wrong enum values from circuit connection status and fix enum name * Update ExpressRouteCrossConnection Route Table Summary Record (#2574) * Update CrossConnection Route Table Summary record * [Network-2018-02-01] DDoS Protection Plan resource (#2567) * Initial version * Add eol * Add extra property in example * Add default values for fields * Rename operation IDs * Make 'id' read-only for all network resources * Revert "Make 'id' read-only for all network resources" * Fix reference * making peer express route circuit peering and express route circuit peering as references (#2696) * Express Route Cross Connection Swagger changes as per PM requirements (#2644) * Fixed operationIds for non-CRUD operations in ExpressRouteCrossConnections (#2720) * Making travis run > 10 mins (#2739)

gimotwanMSFT added 3 commits February 22, 2018 15:54

Fix whitespace

9865728

Fix incorrect naming

b321117

Merge remote-tracking branch 'upstream/Network-2018-02-01' into Netwo…

89347dd

…rk-2018-02-01

dsgouda suggested changes Feb 26, 2018

View reviewed changes

Address review comments

a4e7d6a

dsgouda suggested changes Feb 26, 2018

View reviewed changes

gimotwanMSFT added 2 commits February 26, 2018 15:40

Address Review comments

5f951d5

Revert change to mark service provider properties object as read-only

04d975c

dsgouda suggested changes Feb 26, 2018

View reviewed changes

dsgouda reviewed Feb 26, 2018

View reviewed changes

dsgouda suggested changes Feb 26, 2018

View reviewed changes

gimotwanMSFT added 2 commits February 26, 2018 16:03

Fix Indentation issues

2c8e13f

ExpressRouteServiceProviderProperties is readOnly

4dc9152

dsgouda suggested changes Feb 27, 2018

View reviewed changes

dsgouda self-assigned this Feb 27, 2018

Add Examples for ExpressRouteCrossConnection Operations

dc34be0

Add examples for ExpressRouteCrossConnection Operations

e818f8d

dsgouda approved these changes Mar 1, 2018

View reviewed changes

dsgouda merged commit 62017bc into Azure:Network-2018-02-01 Mar 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Express Route Provider APIs #2532

Add support for Express Route Provider APIs #2532

gimotwanMSFT commented Feb 23, 2018

AutorestCI commented Feb 23, 2018 •

edited

Loading

AutorestCI commented Feb 23, 2018 •

edited

Loading

dsgouda left a comment

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

gimotwanMSFT Feb 26, 2018

gimotwanMSFT Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

gimotwanMSFT Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

gimotwanMSFT Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda left a comment

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda left a comment

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda Feb 26, 2018

dsgouda left a comment

dsgouda commented Feb 27, 2018

gimotwanMSFT commented Mar 1, 2018

dsgouda commented Mar 1, 2018

dsgouda left a comment

Add support for Express Route Provider APIs #2532

Add support for Express Route Provider APIs #2532

Conversation

gimotwanMSFT commented Feb 23, 2018

AutorestCI commented Feb 23, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Feb 23, 2018 • edited Loading

Automation for azure-sdk-for-go

dsgouda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsgouda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsgouda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsgouda left a comment

Choose a reason for hiding this comment

dsgouda commented Feb 27, 2018

gimotwanMSFT commented Mar 1, 2018

dsgouda commented Mar 1, 2018

dsgouda left a comment

Choose a reason for hiding this comment

AutorestCI commented Feb 23, 2018 •

edited

Loading

AutorestCI commented Feb 23, 2018 •

edited

Loading