Adding Detector Endpoints for Web Apps and App Service Environments #2764

stevenernst131 · 2018-03-28T18:36:42Z

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.

There were errors and warnings that already exist in the Microsoft.Web provider, but there are no new errors and warnings introduced.

AutorestCI · 2018-03-28T18:43:45Z

Automation for azure-sdk-for-python

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-python#2065

AutorestCI · 2018-03-28T18:49:48Z

Automation for azure-libraries-for-java

The initial PR has been merged into your service PR:
AutorestCI/azure-libraries-for-java#4

AutorestCI · 2018-03-28T18:57:20Z

Automation for azure-sdk-for-go

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-go#1624

AutorestCI · 2018-03-28T19:34:09Z

Automation for azure-sdk-for-node

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-node#2612

jianghaolu · 2018-03-29T21:21:59Z

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

+      "type": "object",
+      "properties": {
+        "description": {
+          "description": "Description",


Please make this a little more descriptive :)

jianghaolu · 2018-03-29T21:22:13Z

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

+          "type": "string"
+        },
+        "description": {
+          "description": "Description",


jianghaolu · 2018-03-29T21:22:57Z

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

@@ -15,6 +15,236 @@
    "application/json"
  ],
  "paths": {
+    "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Web/hostingEnvironments/{name}/detectors": {


Please add examples for these new operations.

There are 4 new endpoints, 6 if you include the slot variations, and they all have examples. Let me know if you are not seeing one though.

Sorry for the late response - You need to reference them through "x-ms-examples" on the operation. https://github.com/Azure/azure-rest-api-specs/blob/master/documentation/x-ms-examples.md

Hi @jianghaolu, thanks for the resource, I have added the references to the examples.

Thanks @stevenernst131 I'll take a look at the examples now

stevenernst131 · 2018-03-29T23:37:01Z

@jianghaolu I have added the richer descriptions and pushed the change.

jianghaolu · 2018-04-03T18:09:43Z

...rosoft.Web/stable/2016-03-01/examples/Diagnostics_GetHostingEnvironmentDetectorResponse.json

+    "responses": {
+        "200": {
+            "body": {
+                "value": {


Please move everything under "value" to under "body". The Swagger definition doesn't contain a "value" for DetectorResponse.

Thanks for catching, I think this was a copy paste error. I have removed value in all 3 places

This one, however, should have "value" removed.

jianghaolu · 2018-04-03T18:36:36Z

...-manager/Microsoft.Web/stable/2016-03-01/examples/Diagnostics_ListSiteDetectorResponses.json

+                                "subCategory": "App Availability",
+                                "supportTopicId": ""
+                            },
+                            "dataset": [


dataset is defined as an object in type DetectorResponse.

dataset is an array, I have updated it in the definition. It is an array in all examples.

jianghaolu · 2018-04-03T18:37:05Z

...ce-manager/Microsoft.Web/stable/2016-03-01/examples/Diagnostics_GetSiteDetectorResponse.json

+    "responses": {
+        "200": {
+            "body": {
+                "value": {


Please remove "value".

jianghaolu · 2018-04-03T18:37:25Z

...ager/Microsoft.Web/stable/2016-03-01/examples/Diagnostics_ListSiteDetectorResponsesSlot.json

+                                "subCategory": "App Availability",
+                                "supportTopicId": ""
+                            },
+                            "dataset": [


"dataset" is not a list.

Changed to list.

jianghaolu · 2018-04-03T18:37:41Z

...anager/Microsoft.Web/stable/2016-03-01/examples/Diagnostics_GetSiteDetectorResponseSlot.json

+    "responses": {
+        "200": {
+            "body": {
+                "value": {


Please remove "value".

jianghaolu · 2018-04-03T19:51:28Z

...soft.Web/stable/2016-03-01/examples/Diagnostics_ListHostingEnvironmentDetectorResponses.json

-                        }
-                    }
-                ]
+                "id": "/subscriptions/34adfa4f-cedf-4dc0-ba29-b6d1a69ab345/resourceGroups/Sample-WestUSResourceGroup/providers/Microsoft.Web/hostingEnvironments/SampleAppServiceEnvironment/detectors/runtimeavailability",


the "value" here is actually correct since this is from a list operation.

Removed it from the wrong file, added it back to this file and removed it from Diagnostics_GetHostingEnvironmentDetectorResponse.

jianghaolu · 2018-04-03T19:52:49Z

...ce-manager/Microsoft.Web/stable/2016-03-01/examples/Diagnostics_GetSiteDetectorResponse.json

+            "body": {
+                "id": "/subscriptions/ef90e930-9d7f-4a60-8a99-748e0eea69de/resourceGroups/Build2015DemoRG/providers/Microsoft.Web/sites/BuggyBakery/detectors/runtimeavailability",
+                "name": "runtimeavailability",
+                "location": "West US",


"location" is not allowed for a proxy resource.

jianghaolu · 2018-04-03T19:55:18Z

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

+              "type": "array",
+              "description": "Data Set",
+              "items": {
+                "$ref": "#/definitions/Solution"


Why did you change the item type from DiagnosticData to Solution?

Copy paste error, corrected.

stevenernst131 · 2018-04-04T21:59:41Z

Hi @jianghaolu, I believe I have addressed all your issues. Let me know if there is anything else from your side to be addressed.

jianghaolu · 2018-04-06T16:37:33Z

...rosoft.Web/stable/2016-03-01/examples/Diagnostics_GetHostingEnvironmentDetectorResponse.json

+                                ]
+                            },
+                            "renderingProperties": {
+                                "renderingType": 2,


This is defined as an enum but this is an integer. Same elsewhere.

I have changed it to return a string, which is the actual way that we return enums.

jianghaolu · 2018-04-06T16:38:49Z

@stevenernst131 Sorry for the late response. One more issue and we should be good to go. (The new issue was in Dataset which was invalid earlier so the tool didn't report it earlier)

jianghaolu · 2018-04-06T16:41:32Z

Oh I just realized you changed all the error response types - that's a breaking change - let me sync with Samer to see what to do here.

Also fixing enum

stevenernst131 · 2018-04-06T20:32:40Z

Hi @jianghaolu, I did not mean to change the Error Response in a way that caused a breaking change, that was not my intention. I have changed it back to how it was before, so let me know if that is correct.

stevenernst131 · 2018-04-10T17:28:02Z

@jianghaolu Please let me know if there is anything else I need to do.

jianghaolu · 2018-04-12T21:07:58Z

@stevenernst131 Hi Steve! Sorry for the late response. Been crazy busy with other deliverables.

Actually, do you think you can just remove those added "default" sections? Our SDKs default to throw CloudException when a default section is not provided. Changing them to CommonDefinitions.json#/definitions/DefaultErrorResponse is also a breaking change.

stevenernst131 · 2018-04-12T21:17:25Z

@jianghaolu Thanks for the response, I have pushed a change to remove all the default sections, let me know if that works for you.

azuresdkciprbot · 2018-04-12T22:07:23Z

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): 0 Error(s): 26
After the PR: Warning(s): 0 Error(s): 26

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-12T22:47:25Z

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:

💡 Please review potentially introduced Error(s)/Warning(s): Analysis Report 💡

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

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

jianghaolu · 2018-04-16T23:49:55Z

Thanks! Merging!

Adding new detector endpoint

010e425

azuresdkci assigned jianghaolu Mar 28, 2018

azuresdkci requested a review from jianghaolu March 28, 2018 18:36

AutorestCI mentioned this pull request Mar 28, 2018

[AutoPR web/resource-manager] Adding Detector Endpoints for Web Apps and App Service Environments Azure/azure-sdk-for-python#2295

Closed

AutorestCI mentioned this pull request Mar 28, 2018

[AutoPR web/resource-manager] Adding Detector Endpoints for Web Apps and App Service Environments Azure/azure-sdk-for-node#2579

Closed

jianghaolu suggested changes Mar 29, 2018

View reviewed changes

Adding richer descriptions

fc77347

Adding Examples

4ea600f

jianghaolu suggested changes Apr 3, 2018

View reviewed changes

Remove value and make data set array

1e66b0c

jianghaolu suggested changes Apr 3, 2018

View reviewed changes

jianghaolu reviewed Apr 3, 2018

View reviewed changes

Fixing Issues

5710401

jianghaolu reviewed Apr 6, 2018

View reviewed changes

Undoing Error Response Change

05d0cc0

Also fixing enum

Removing Default Sections

9e43026

AutorestCI mentioned this pull request Apr 12, 2018

[AutoPR web/resource-manager] Adding Detector Endpoints for Web Apps and App Service Environments Azure/azure-sdk-for-go#1575

Closed

jianghaolu approved these changes Apr 16, 2018

View reviewed changes

jianghaolu merged commit 8e57299 into Azure:master Apr 16, 2018

AutorestCI mentioned this pull request Apr 16, 2018

[AutoPR web/resource-manager] Adding Detector Endpoints for Web Apps and App Service Environments Azure/azure-sdk-for-go#1623

Merged

Adding Detector Endpoints for Web Apps and App Service Environments #2764

Adding Detector Endpoints for Web Apps and App Service Environments #2764

Conversation

stevenernst131 commented Mar 28, 2018 • edited by azuresdkci Loading

PR information

Quality of Swagger

AutorestCI commented Mar 28, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Mar 28, 2018 • edited Loading

Automation for azure-libraries-for-java

AutorestCI commented Mar 28, 2018 • edited Loading

Automation for azure-sdk-for-go

AutorestCI commented Mar 28, 2018 • edited Loading

Automation for azure-sdk-for-node

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

stevenernst131 commented Mar 29, 2018

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

stevenernst131 commented Apr 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianghaolu commented Apr 6, 2018

jianghaolu commented Apr 6, 2018 • edited Loading

stevenernst131 commented Apr 6, 2018

stevenernst131 commented Apr 10, 2018

jianghaolu commented Apr 12, 2018

stevenernst131 commented Apr 12, 2018

azuresdkciprbot commented Apr 12, 2018

azuresdkciprbot commented Apr 12, 2018

jianghaolu commented Apr 16, 2018

stevenernst131 commented Mar 28, 2018 •

edited by azuresdkci

Loading

AutorestCI commented Mar 28, 2018 •

edited

Loading

AutorestCI commented Mar 28, 2018 •

edited

Loading

AutorestCI commented Mar 28, 2018 •

edited

Loading

AutorestCI commented Mar 28, 2018 •

edited

Loading

jianghaolu commented Apr 6, 2018 •

edited

Loading