[ADLS] Adding vnet to read-only account properties and test examples for each API #3055

lewu-msft · 2018-05-10T23:56:02Z

For ADLS management plane, added vnet to read-only account properties. added test examples for each API. fixed some example links.

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.

…amples

azuresdkciprbot · 2018-05-11T00:02:31Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/datalake-analytics/resource-manager/readme.md

⚠️

0 new Warnings.(16 total)

❌0 new Errors.(0 total)

File: specification/datalake-store/resource-manager/readme.md

⚠️

1 new Warnings.(10 total)

Code	Id	Source	Message
TrackedResourceListByImmediateParent	R3010	Link	The child tracked resource, 'virtualNetworkRules' with immediate parent 'DataLakeStoreAccount', must have a list by immediate parent operation.

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-05-11T00:02:31Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/datalake-analytics/resource-manager/readme.md

⚠️

0 new Warnings.(12 total)

❌0 new Errors.(0 total)

File: specification/datalake-store/resource-manager/readme.md

⚠️

1 new Warnings.(8 total)

Code	Id	Source	Message
PutRequestResponseScheme	R2017	Link	A PUT operation request body schema should be the same as its 200 response schema, to allow reusing the same entity between GET and PUT. If the schema of the PUT request body is a superset of the GET response body, make sure you have a PATCH operation to make the resource updatable. Operation: 'VirtualNetworkRules_CreateOrUpdate' Request Model: 'CreateOrUpdateVirtualNetworkRuleParameters' Response Model: 'VirtualNetworkRule'

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

anuchandy · 2018-05-11T00:07:57Z

...ation/datalake-store/resource-manager/Microsoft.DataLakeStore/stable/2016-11-01/account.json

+              "$ref": "#/definitions/VirtualNetworkRule"
+            }
+          }
+        },


Is this a long running operation? if so model this operation as LRO.

anuchandy · 2018-05-11T00:08:08Z

...ation/datalake-store/resource-manager/Microsoft.DataLakeStore/stable/2016-11-01/account.json

+            "schema": {
+              "$ref": "#/definitions/VirtualNetworkRule"
+            }
+          }


Is this a long running operation? if so model this operation as LRO.

anuchandy · 2018-05-11T00:08:46Z

...ation/datalake-store/resource-manager/Microsoft.DataLakeStore/stable/2016-11-01/account.json

+            "description": "The specified virtual network rule does not exist or was already deleted."
+          }
+        },
+        "x-ms-examples": {


Is this a long running operation? if so model this operation as LRO.

thanks. these vnet rule APIs are not async, so I think no need to model as LRO.

anuchandy · 2018-05-11T00:13:33Z

@ravbhatnagar can you please take a look, this PR introduces CRUD on network rules.

lewu-msft · 2018-05-14T21:20:02Z

any more comments are appreciated. Thanks

ravbhatnagar · 2018-05-15T06:35:59Z

...ation/datalake-store/resource-manager/Microsoft.DataLakeStore/stable/2016-11-01/account.json

+        "properties"
+      ],
+      "properties": {
+        "name": {


name MUST come from the URL. It should not be present in the request body. In request body, it should be marked s readonly.

Looks like on the PUT of account, you are providing the option of creating a virtualNetworkRule. This is not supported. VirtualNetworkRule must be explicitly created by doing a PUT on this type. If needed, It can be referenced in the parent resource using the resourceId.

To the second comment, I agree that the most standard way is to only allow the nested resource creation/update through nested resource APIs. However in our service we are providing an additional way that users can also create/update nested resources through the top level resource APIs. This is more a historical thing from the very beginning of our service. For now I guess we still want to keep it to have the consistency with other nested resource types we already have. Going forward in the next major API version upgrade, I think we can consider to go the most standard way.

ravbhatnagar · 2018-05-15T06:37:53Z

...ation/datalake-store/resource-manager/Microsoft.DataLakeStore/stable/2016-11-01/account.json

+            "in": "body",
+            "required": true,
+            "schema": {
+              "$ref": "#/definitions/CreateOrUpdateVirtualNetworkRuleParameters"


The same model can be used between request and response by marking the appropriate properties as readonly if they are not allowed to be set in the request but are present in response.

@ravbhatnagar This is surely one way to design this. The design we took, however, was this type of model for all API requests. Changing it to your suggested model would create inconsistency. This is something that @lewu-msft can consider for a future design revamp.

It makes usability difficult from a client point of view. It makes the GET-PUT pipeline harder from the clients. I am fine with this if the SDK team is ok with this. @anuchandy FYI

ravbhatnagar · 2018-05-18T00:03:25Z

Signing off. One major thing noted in the review was PUT on parent creating the child. This is an antipattern in ARM. But RP has this model for other resource types as well and would want to continue this. In the next api-version, RP should fix this.

AutorestCI · 2018-05-18T22:45:21Z

Automation for azure-sdk-for-node

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

AutorestCI · 2018-05-18T22:45:47Z

Automation for azure-libraries-for-java

A PR has been created for you:
AutorestCI/azure-libraries-for-java#175

AutorestCI · 2018-05-18T22:46:14Z

Automation for azure-sdk-for-python

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

AutorestCI · 2018-05-18T23:45:27Z

Automation for azure-sdk-for-go

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

jianghaolu · 2018-10-05T20:36:51Z

@AutorestCI build azure-sdk-for-java

AutorestCI · 2018-10-05T20:36:53Z

I didn't understand your command:

build azure-sdk-for-java