Adding support for delegations on a subnet by rupalivohra · Pull Request #3693 · Azure/azure-rest-api-specs

rupalivohra · 2018-08-21T21:56:32Z

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

The title of the PR is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
If applicable, the PR references the bug/issue that it fixes.
Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

I have read the contribution guidelines.
My spec meets the review criteria:
- The spec conforms to the Swagger 2.0 specification.
- The spec follows the guidelines described in the Swagger checklist document.
- Validation tools were run on swagger spec(s) and have all been fixed in this PR.

azuresdkci · 2018-08-21T22:00:10Z

Can one of the admins verify this patch?

AutorestCI · 2018-08-21T22:02:28Z

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

AutorestCI · 2018-08-21T22:06:53Z

Automation for azure-sdk-for-python

A PR has been created for you:
Azure/azure-sdk-for-python#3231

AutorestCI · 2018-08-21T22:26:46Z

Automation for azure-sdk-for-go

A PR has been created for you:
Azure/azure-sdk-for-go#2585

AutorestCI · 2018-08-21T22:46:24Z

Automation for azure-sdk-for-node

A PR has been created for you:
Azure/azure-sdk-for-node#3393

AutorestCI · 2018-08-21T22:48:32Z

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

jianghaolu · 2018-08-23T06:41:01Z

...anager/Microsoft.Network/stable/2018-08-01/examples/AvailableDelegationsSubscriptionGet.json

Parameter "location" is required.

...anager/Microsoft.Network/stable/2018-08-01/examples/AvailableDelegationsSubscriptionGet.json

jianghaolu · 2018-08-23T06:42:39Z

...nager/Microsoft.Network/stable/2018-08-01/examples/AvailableDelegationsResourceGroupGet.json

Missing parameter "location".

...nager/Microsoft.Network/stable/2018-08-01/examples/AvailableDelegationsResourceGroupGet.json

jianghaolu · 2018-08-23T06:44:45Z

...ager/Microsoft.Network/stable/2018-08-01/examples/VirtualNetworkGetWithSubnetDelegation.json

According to the spec, "delegations" is a list of ServiceDelegationPropertiesFormat which doesn't contain name, id, properties.

Hey Jianghao - I noticed that "id" isn't included in most of the resources, but is included in the examples (as it is in the response of the request).

Does your service return "id", "name", etc? The spec and examples must match the service behavior.

"provisioningState" in "properties" is not defined in the spec.

Id, name comes from the SubResource.

And provisioningState is included here:
"ServiceDelegationPropertiesFormat": {
"properties": {
"serviceName": {
"type": "string",
"description": "The name of the service to whom the subnet should be delegated (e.g. Microsoft.Sql/servers)"
},
"actions": {
"type": "array",
"items": {
"type": "string"
},
"description": "Describes the actions permitted to the service upon delegation"
},
"provisioningState": {
"type": "string",
"description": "The provisioning state of the resource."
}
},

...k/resource-manager/Microsoft.Network/stable/2018-08-01/examples/SubnetGetWithDelegation.json

...esource-manager/Microsoft.Network/stable/2018-08-01/examples/SubnetCreateWithDelegation.json

chandrasekarsrinivasan · 2018-08-24T00:25:32Z

...ation/network/resource-manager/Microsoft.Network/stable/2018-08-01/availableDelegations.json

Is this a special character?

looks like it just denotes that there is no newline at the EOF. Since none of the other files are saying that, I'll just add a newline

...ation/network/resource-manager/Microsoft.Network/stable/2018-08-01/availableDelegations.json

tjprescott · 2018-08-30T17:46:33Z

specification/network/resource-manager/Microsoft.Network/stable/2018-08-01/virtualNetwork.json

You need to include the "purpose" property as a read-only property or the SDK (and thus CLI) will not see it.

jianghaolu

Most errors are repeated across examples - I'm not going to post them all since they could be spec errors. Once spec errors are fixed we may see a significant decrease in model validation errors.

...anager/Microsoft.Network/stable/2018-08-01/examples/AvailableDelegationsSubscriptionGet.json

...nager/Microsoft.Network/stable/2018-08-01/examples/AvailableDelegationsResourceGroupGet.json

jianghaolu · 2018-08-31T18:24:41Z

...ager/Microsoft.Network/stable/2018-08-01/examples/VirtualNetworkGetWithSubnetDelegation.json

Does your service return "id", "name", etc? The spec and examples must match the service behavior.

jianghaolu · 2018-08-31T18:27:39Z

...esource-manager/Microsoft.Network/stable/2018-08-01/examples/SubnetCreateWithDelegation.json

According the spec you are returning a "Delegation" instead of an "AvailableDelegation" here. "Delegation"'s properties don't contain provisioningState and `actions': https://github.com/Azure/azure-rest-api-specs/pull/3693/files#diff-48ae68150e376ac636b1bc2e92b09b32R868.

Maybe the service is actually returning "AvailableDelegation"? Please double check.

AvailableDelegation and ServiceDelegation are two different resources on the backend

Jianghaolu - I do not see any problem here. Rupali is returning Delegations. Can you please confirm and explain more on what you are saying?

jianghaolu · 2018-08-31T20:44:48Z

...ation/network/resource-manager/Microsoft.Network/stable/2018-08-01/availableDelegations.json

You should extend this from https://github.com/Azure/azure-rest-api-specs/blob/master/specification/common-types/resource-management/v1/types.json#L78.

Couple of questions:

Why?

How?

Because if it IS a proxy resource then extending it from proxy resource is the CORRECT way.

You may also extend from SubResource and get rid of the id property, that's okay too

We'll fix this and get back.

jianghaolu · 2018-09-05T00:46:12Z

specification/network/resource-manager/Microsoft.Network/stable/2018-08-01/virtualNetwork.json

+    },
+    "Delegation": {
+      "properties": {
+        "properties": {


Property missing: "type"

jianghaolu · 2018-09-05T00:48:14Z

...ager/Microsoft.Network/stable/2018-08-01/examples/VirtualNetworkGetWithSubnetDelegation.json

+                ],
+                "purpose": ""
+              },
+              "type": "Microsoft.Network/virtualNetworks/subnets"


"type" property is also missing in the definition of "Subnet"

rupalivohra · 2018-09-10T18:44:42Z

This PR was completed via #3805
Closing this PR.

rupalivohra requested a review from MikhailTryakhov as a code owner August 21, 2018 21:56

azuresdkci assigned jianghaolu Aug 21, 2018

azuresdkci requested a review from jianghaolu August 21, 2018 21:57

rupalivohra mentioned this pull request Aug 21, 2018

Adding Delegations to Subnets #3666

Closed

10 tasks

rupalivohra force-pushed the Network-September-Release branch from c1cfa85 to 0985af5 Compare August 21, 2018 23:54

AutorestCI mentioned this pull request Aug 22, 2018

[AutoPR migrate/resource-manager/network/resource-manager] Adding support for delegations on a subnet Azure/azure-sdk-for-node#3393

Closed

rupalivohra force-pushed the Network-September-Release branch 4 times, most recently from 6b10c04 to 8ef462d Compare August 22, 2018 23:11

jianghaolu suggested changes Aug 23, 2018

View reviewed changes

chandrasekarsrinivasan reviewed Aug 24, 2018

View reviewed changes

...ation/network/resource-manager/Microsoft.Network/stable/2018-08-01/availableDelegations.json Outdated Show resolved Hide resolved

chandrasekarsrinivasan reviewed Aug 24, 2018

View reviewed changes

...ation/network/resource-manager/Microsoft.Network/stable/2018-08-01/availableDelegations.json Outdated Show resolved Hide resolved

rupalivohra force-pushed the Network-September-Release branch from 8ef462d to 222ffaf Compare August 30, 2018 02:15

AutorestCI mentioned this pull request Aug 30, 2018

[AutoPR network/resource-manager] Adding support for delegations on a subnet Azure/azure-sdk-for-python#3231

Closed

tjprescott reviewed Aug 30, 2018

View reviewed changes

rupalivohra force-pushed the Network-September-Release branch 2 times, most recently from 7fec20d to 09faf04 Compare August 30, 2018 19:14

AutorestCI mentioned this pull request Aug 30, 2018

[AutoPR network/resource-manager] Adding support for delegations on a subnet Azure/azure-sdk-for-go#2585

Closed

rupalivohra force-pushed the Network-September-Release branch 3 times, most recently from 8a7d951 to a35f946 Compare August 31, 2018 02:21

jianghaolu suggested changes Aug 31, 2018

View reviewed changes

Rupali Vohra added 3 commits August 31, 2018 11:37

Adding subnet delegation property on VNET

85415c7

Updating subnet/vnet create & get examples to reflect delegation

260c04a

Adding support for GetAvailableDelegations operation

ea8045e

rupalivohra force-pushed the Network-September-Release branch from a35f946 to ea8045e Compare August 31, 2018 18:49

jianghaolu reviewed Aug 31, 2018

View reviewed changes

Fixing properties in Swagger & updating examples to reflect fixes

0139e0a

rupalivohra force-pushed the Network-September-Release branch from 21773b5 to 0139e0a Compare August 31, 2018 21:23

jianghaolu reviewed Sep 5, 2018

View reviewed changes

rupalivohra closed this Sep 10, 2018

Conversation

rupalivohra commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR information

Quality of Swagger

Uh oh!

azuresdkci commented Aug 21, 2018

Uh oh!

AutorestCI commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-ruby

Uh oh!

AutorestCI commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-python

Uh oh!

AutorestCI commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-go

Uh oh!

AutorestCI commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-node

Uh oh!

AutorestCI commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-java

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jianghaolu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

jianghaolu Aug 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

rupalivohra commented Aug 21, 2018 •

edited

Loading

AutorestCI commented Aug 21, 2018 •

edited

Loading

AutorestCI commented Aug 21, 2018 •

edited

Loading

AutorestCI commented Aug 21, 2018 •

edited

Loading

AutorestCI commented Aug 21, 2018 •

edited

Loading

AutorestCI commented Aug 21, 2018 •

edited

Loading

jianghaolu Aug 31, 2018 •

edited

Loading