Added new top level resource for verifying VNET for ASE by hforeste · Pull Request #1574 · Azure/azure-rest-api-specs

hforeste · 2017-08-23T00:02:41Z

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.

msftclas · 2017-08-23T00:02:45Z

@hforeste,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

sergey-shandar · 2017-08-23T19:58:19Z

specification/web/resource-manager/Microsoft.Web/2016-03-01/ResourceProvider.json

    },
+    "/subscriptions/{subscriptionId}/providers/Microsoft.Web/verifyHostingEnvironmentVnet": {
+      "post": {
+        "summary": "Verifies if this VNET is compatible with an App Service Environment by analyzing the Network Security Group rules",


'summary' and 'description' shouldn't be the same.

'summary' is a short summary of what operation does.

'description' is a verbose explanation.

I shortened the summary in the next iteration.
Our tooling auto generates this swagger file hence why the summary and description are the same. Similar to the other APIs in the file.

sergey-shandar · 2017-08-23T20:10:06Z

specification/web/resource-manager/Microsoft.Web/2016-03-01/ResourceProvider.json

+        "operationId": "VerifyHostingEnvironmentVnet",
+        "consumes": [
+          "application/json",
+          "text/json",


Does your service support all of these types? AFAIK, ARM recommends only "allpication/json".

sergey-shandar · 2017-08-23T20:21:16Z

specification/web/resource-manager/Microsoft.Web/2016-03-01/ResourceProvider.json

        }
      }
    },
+    "Object": {


Is there a reason to create an alias for an object type?

sergey-shandar · 2017-08-23T20:23:55Z

specification/web/resource-manager/Microsoft.Web/2016-03-01/ResourceProvider.json

+          "200": {
+            "description": "OK",
+            "schema": {
+              "$ref": "#/definitions/Object"


Is it possible to provide more information about what is the response structure? For example, does it have some common properties?

sergey-shandar · 2017-08-23T20:26:18Z

specification/web/resource-manager/Microsoft.Web/2016-03-01/ResourceProvider.json

      }
+    },
+    "VnetParameters": {
+      "description": "",


Could you provide a 'description' for the type? The description is used to generate API comments and documentation so it is useful for our customers.

sergey-shandar

Could you provide x-ms-example(s) for the new operation?

sergey-shandar · 2017-08-24T18:24:08Z

specification/web/resource-manager/Microsoft.Web/2016-03-01/ResourceProvider.json

+          "text/json",
+          "application/xml",
+          "text/xml"
+        ],


@hforeste still the same question, does your service support all of the content types (text/json, application/xml)? If supported content type are the same, could you place them to the top level of the swagger?
@ravbhatnagar are you ok with these content types?

@sergey-shandar , our tooling automatically adds these content types. We're planning to fix this here in the near future.

@hforeste Excellent! Do you have a ticket for this issue so we can track it?

I'll have to check with @naveedaz, not sure where this work item might be tracked. Navy, do you have a work item tracked for this?

@hforeste - ARM does not support content types other than application/json. So, please remove them. Even though these are generated by a tool, will it be a problem if you manually fix these?

No, it should not be a problem. Thanks!

sergey-shandar · 2017-08-24T18:26:08Z

specification/web/resource-manager/Microsoft.Web/2016-03-01/ResourceProvider.json

+        "properties": {
+          "description": "VnetValidationFailureDetails resource specific properties",
+          "properties": {
+            "failed": {


Could you add description to the property?

sergey-shandar · 2017-08-24T18:26:44Z

specification/web/resource-manager/Microsoft.Web/2016-03-01/ResourceProvider.json

+              "type": "boolean"
+            },
+            "failedTests": {
+              "type": "array",


same here: description

sergey-shandar · 2017-08-24T18:27:29Z

specification/web/resource-manager/Microsoft.Web/2016-03-01/ResourceProvider.json

+            "testName": {
+              "type": "string"
+            },
+            "details": {


same here, 'description'

hforeste · 2017-08-25T17:42:21Z

Could you provide x-ms-example(s) for the new operation?
->I think we can address this afterwards.

sergey-shandar · 2017-08-28T18:32:00Z

@hforeste do you need help with examples?

sergey-shandar · 2017-08-29T17:35:25Z

...ion/web/resource-manager/Microsoft.Web/2016-03-01/examples/VerifyHostingEnvironmentVnet.json

+      "api-version": "2016-03-01",
+      "vNetParameters": {
+        "properties": { 
+            "vnetResourceGroup": 'vNet123rg',


JSON errors. Please, use double quotes "".

sergey-shandar · 2017-08-29T18:04:21Z

...ion/web/resource-manager/Microsoft.Web/2016-03-01/examples/VerifyHostingEnvironmentVnet.json

+  "parameters": {
+      "subscriptionId": "34adfa4f-cedf-4dc0-ba29-b6d1a69ab345",
+      "api-version": "2016-03-01",
+      "vNetParameters": {


the property should be called "parameters".

azuresdkciprbot · 2017-08-29T21:27:05Z

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/web/resource-manager/readme.md
Before the PR: Warning(s): 1637 Error(s): 98
After the PR: Warning(s): 1641 Error(s): 98

AutoRest Linter Guidelines | AutoRest Linter Issues

Send feedback and make AutoRest Linter Azure Bot smarter day by day!

Thanks for your co-operation.

azuresdkciprbot · 2017-08-29T22:16:54Z

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/web/resource-manager/readme.md
Before the PR: Warning(s): 1637 Error(s): 98
After the PR: Warning(s): 1637 Error(s): 98

AutoRest Linter Guidelines | AutoRest Linter Issues

Send feedback and make AutoRest Linter Azure Bot smarter day by day!

Thanks for your co-operation.

AutorestCI · 2017-08-29T22:20:57Z

No modification for AutorestCI/azure-sdk-for-node

AutorestCI · 2017-08-29T22:21:08Z

Azure/azure-sdk-for-python#1400

AutorestCI · 2017-08-29T22:22:03Z

Azure/azure-sdk-for-ruby#934

AutorestCI · 2017-08-30T05:30:21Z

No modification for AutorestCI/azure-sdk-for-node

AutorestCI · 2017-08-30T05:31:11Z

No modification for AutorestCI/azure-sdk-for-ruby

Added new resource provider for VNET verify

1d2cb7b

msftclas added the cla-not-required label Aug 23, 2017

azuresdkci assigned sergey-shandar Aug 23, 2017

sergey-shandar suggested changes Aug 23, 2017

View reviewed changes

Address pull request comments

c5d5661

sergey-shandar suggested changes Aug 24, 2017

View reviewed changes

sergey-shandar added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Aug 24, 2017

Address pull requests comments

f547cf9

ravbhatnagar removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Aug 29, 2017

hforeste added 3 commits August 29, 2017 10:10

Add examples for VerifyHostingEnvironmentVnet

8f1c3cc

Add example tag to resourceprovider.json

e2f732c

Remove all content types except application/json

f48165b

sergey-shandar suggested changes Aug 29, 2017

View reviewed changes

fix json errors

7bd7dc9

sergey-shandar reviewed Aug 29, 2017

View reviewed changes

Respond to comments

103e0fe

Update ResourceProvider.json

403b56e

sergey-shandar approved these changes Aug 29, 2017

View reviewed changes

sergey-shandar merged commit 45c6e27 into Azure:current Aug 29, 2017

AutorestCI mentioned this pull request Aug 29, 2017

Automatic PR from RestAPI-PR1574 Azure/azure-sdk-for-python#1400

Closed

AutorestCI mentioned this pull request Aug 29, 2017

Automatic PR from RestAPI-PR1574 Azure/azure-sdk-for-ruby#934

Merged

veronicagg mentioned this pull request Sep 11, 2017

[azure_mgmt_*] Prepare to release azure_mgmt_* version 0.12.0 Azure/azure-sdk-for-ruby#962

Merged

9 tasks

Conversation

hforeste commented Aug 23, 2017 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR information

Quality of Swagger

Uh oh!

msftclas commented Aug 23, 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!

sergey-shandar left a comment

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!

sergey-shandar Aug 24, 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!

Choose a reason for hiding this comment

Uh oh!

hforeste commented Aug 25, 2017

Uh oh!

sergey-shandar commented Aug 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azuresdkciprbot commented Aug 29, 2017

Uh oh!

azuresdkciprbot commented Aug 29, 2017

Uh oh!

AutorestCI commented Aug 29, 2017

Uh oh!

AutorestCI commented Aug 29, 2017

Uh oh!

AutorestCI commented Aug 29, 2017

Uh oh!

AutorestCI commented Aug 30, 2017

Uh oh!

AutorestCI commented Aug 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

hforeste commented Aug 23, 2017 •

edited by azuresdkci

Loading

sergey-shandar Aug 24, 2017 •

edited

Loading