[DO-NOT-MERGE] Add new resource type /events for Azure service health events by HimanshuMisra1 · Pull Request #2127 · Azure/azure-rest-api-specs

HimanshuMisra1 · 2017-12-09T00:47:04Z

Resource Health RP changes for new resource type /events.
This resource type will return all service health events (incidents/maintenance/advisories) that are either declared or detected from automated service monitoring using Geneva Health System.

olydis · 2017-12-09T01:11:17Z

@HimanshuMisra1 please pull the Swagger file out of the swagger folder (see typical folder structure of this repo)

olydis · 2017-12-09T01:11:27Z

@ravbhatnagar new Swagger

…sources by the service health event

olydis · 2017-12-13T17:41:54Z

...source-manager/Microsoft.ResourceHealth/2018-01-01/examples/Events_ListBySubscriptionId.json

+						"name": "VIRTUALMACHINES-WESTUS-DTR1",
+						"type": "/providers/Microsoft.ResourceHealth/events",
+						"properties": {
+							"eventType": "serviceissue",


The allowed values for eventType and eventSource have a different spelling than specified here - please adjust either the enum definitions or the example (to match your actual service as it is deployed right now)

Changed the examples to use correct values from enum.

olydis · 2017-12-13T18:04:56Z

Please adjust the readme.md of this RP to point to the new Swagger (note the slightly altered spelling of your Swagger, our tooling is case sensitive) 🙂

olydis · 2017-12-13T18:08:35Z

...tion/resourcehealth/resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json

+        ],
+        "responses": {
+          "200": {
+            "description": "The body contains the list of the current service health events in the subscription",


Nit: Replace The body contains the with The in this Swagger - since this description makes it into high level artifacts like SDKs and such, descriptions should rather not talk about REST technicalities. 🙂

olydis · 2017-12-13T18:10:23Z

...tion/resourcehealth/resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json

+        },
+        "nextLink": {
+          "type": "string",
+          "description": "The URI to fetch the next page of events. Call ListNext() with this URI to fetch the next page of events."


same as mentioned above: please remove part about "ListNext" - it is actually not called that way in some languages others even retrieve pages on the fly.

Fixed, Thanks

olydis · 2017-12-13T18:15:23Z

...tion/resourcehealth/resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json

+      ],
+      "description": "The List events operation response."
+    },
+    "event": {


Is this a resource? If so, please model it as such. See for example https://github.com/Azure/azure-rest-api-specs/blob/current/specification/compute/resource-manager/Microsoft.Compute/2017-12-01/compute.json#L3509-L3526. You actually don't have to copy the definition of Resource into your Swagger but can reference the common definition in https://github.com/Azure/azure-rest-api-specs/blob/current/specification/common-types/resource-management/v1/types.json 🙂

olydis · 2017-12-13T18:31:55Z

...tion/resourcehealth/resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json

+          "description": "Microsoft.ResourceHealth/events"
+        },
+        "properties": {
+          "type": "object",


Usually properties of resources are flattened, as you can also see here (slightly better user experience)

Fixed, Thanks

olydis · 2017-12-13T18:35:03Z

...tion/resourcehealth/resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json

+              "enum": [
+                "Available",
+                "Unavailable",
+                "Degraded",


Added enum value. Breaking change.

What is action item on us here?

Not necessarily anything, but:

any SDKs/CLIs derived from this Swagger will have to bump their major version

you may wanna consider changing modelAsString to true since that would make enum-additions no breaking change in the future

Got it, Thanks

olydis

I assume ARM will also have comments, but here is some feedback for the time being.

olydis · 2017-12-18T10:37:28Z

...tion/resourcehealth/resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json

+      },
+      "allOf": [
+        {
+          "$ref": "#/definitions/Resource"


just $ref ../../../../common-types/resource-management/v1/types.json#/definitions/Resource here, then you can remove the definition of Resource above (which is also slightly invalid since you $ref the entire file "types.json" up there, but really you just want to extract /definitions/Resource from that file) 🙂

simongdavies · 2017-12-19T22:52:02Z

...tion/resourcehealth/resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json

+          "description": "List subscription impacted by the service health event.",
+          "items": {
+            "type": "string",
+            "description": "Subscription impacted by the service health event."


I don't understand why there is a list of subscriptions in here, the events are returned at subscription level, either this is not required or the API is returning data that it should not? Is the intent that there should be a tenant level API where this is returned?

Yes, the intent is to have a tenant level API. The objects returned are tenant level objects and will talk about multiple subscriptions. Since that feature in ARM will be ready by April, 2018, we have to live with exposing single subscription requests. The array in such case will contain only one subscription.

simongdavies · 2017-12-19T22:53:21Z

...tion/resourcehealth/resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json

+        "properties": {
+          "type": "object",
+          "description": "Properties of availability state.",
+          "properties": {


Looks like there is an extra properties object here?

I don't think so.

… definition in common types

marstr · 2018-01-03T22:17:16Z

...tion/resourcehealth/resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json

@@ -0,0 +1,876 @@
+{


In accordance with the new format, introduced late December 2017, please update this PR so that the "2018-01-01" directory is in either a folder named "stable" or "preview". e.g.

resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json -> resource-manager/Microsoft.ResourceHealth/preview/2018-01-01/ResourceHealth.json

or

resource-manager/Microsoft.ResourceHealth/2018-01-01/ResourceHealth.json -> resource-manager/Microsoft.ResourceHealth/preview/2018-01-01/ResourceHealth.json

azuresdkciprbot · 2018-01-10T19:20:15Z

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/resourcehealth/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 1
After the PR: Warning(s): 0 Error(s): 1

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-01-10T19:24:07Z

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/resourcehealth/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 1
After the PR: Warning(s): 0 Error(s): 1

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

ravbhatnagar · 2018-01-10T23:06:06Z

Signed off!

olydis

model validation error

olydis · 2018-01-11T17:45:21Z

...osoft.ResourceHealth/preview/2018-01-01/examples/ImpactedResources_ListBySubscriptionId.json

+						"id": "<resourceId>/providers/Microsoft.ResourceHealth/impactedResources/current",
+						"name": "current",
+						"type": "Microsoft.ResourceHealth/impactedResources",
+						"location": "eastus",


having location here indicates that what you return is actually a tracked resource. Please make sure or adjust the example accordingly

olydis · 2018-01-17T22:38:12Z

@HimanshuMisra1 What's the timeline for this? Is this marked as "do not merge" because the service isn't live yet?

olydis · 2018-01-26T22:49:27Z

@salameer @HimanshuMisra1 closing due to inactivity. Feel free to reopen once you're ready to move on 🙂

AutorestCI · 2018-01-26T22:49:50Z

This commit was treated and no generation was made for Azure/azure-sdk-for-go

AutorestCI · 2018-01-26T22:49:51Z

Swagger to SDK encountered an unknown error: (Azure/azure-sdk-for-go)

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/github_tools.py", line 29, in exception_to_github
    yield context
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 180, in rest_handle_action
    return rest_pull_close(body, github_con, restapi_repo, sdk_pr_target_repo, sdkbase)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 311, in rest_pull_close
    rest_pr.create_issue_comment("Was unable to create SDK %s PR for this closed PR.", sdkid)
TypeError: create_issue_comment() takes 2 positional arguments but 3 were given

AutorestCI · 2018-01-26T22:49:54Z

This commit was treated and no generation was made for Azure/azure-sdk-for-python

AutorestCI · 2018-01-26T22:49:55Z

Swagger to SDK encountered an unknown error: (Azure/azure-sdk-for-python)

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/github_tools.py", line 29, in exception_to_github
    yield context
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 180, in rest_handle_action
    return rest_pull_close(body, github_con, restapi_repo, sdk_pr_target_repo, sdkbase)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 311, in rest_pull_close
    rest_pr.create_issue_comment("Was unable to create SDK %s PR for this closed PR.", sdkid)
TypeError: create_issue_comment() takes 2 positional arguments but 3 were given

Add new resource type /events for Azure service health events

412b0ed

azuresdkci assigned olydis Dec 9, 2017

olydis added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Dec 9, 2017

HimanshuMisra1 added 2 commits December 8, 2017 17:49

Moved swagger file out of swagger folder

08477e1

Add examples

9ce8d46

Azure deleted a comment from azuresdkciprbot Dec 11, 2017

Add /impactedResources RT for resources current status of impacted re…

b2d6dd1

…sources by the service health event

Azure deleted a comment from azuresdkciprbot Dec 13, 2017

olydis reviewed Dec 13, 2017

View reviewed changes

olydis added the In-Review label Dec 13, 2017

olydis reviewed Dec 13, 2017

View reviewed changes

olydis added potential-sdk-breaking-change and removed In-Review labels Dec 13, 2017

olydis suggested changes Dec 13, 2017

View reviewed changes

Addressed PR comments

cbdb3ec

olydis reviewed Dec 18, 2017

View reviewed changes

simongdavies suggested changes Dec 19, 2017

View reviewed changes

Addressed comments: 1. Model enum as string 2. Direct ref to Resource…

df5610d

… definition in common types

marstr changed the base branch from current to master December 28, 2017 17:55

marstr requested changes Jan 3, 2018

View reviewed changes

HimanshuMisra1 added 3 commits January 4, 2018 11:27

Move spec inside 'preview' directory

f5940b8

Move version 2018-01-01 inside preview directory

2f561b3

Adjust relative path to common types

878e1a6

marstr approved these changes Jan 4, 2018

View reviewed changes

HimanshuMisra1 added 2 commits January 10, 2018 11:15

Addressed comments from ARM

a5ad49b

Add correct definition for impacted resources response

2d86e46

ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Jan 10, 2018

HimanshuMisra1 changed the title ~~Add new resource type /events for Azure service health events~~ [DO-NOT-MERGE] Add new resource type /events for Azure service health events Jan 10, 2018

olydis approved these changes Jan 11, 2018

View reviewed changes

olydis suggested changes Jan 11, 2018

View reviewed changes

olydis closed this Jan 26, 2018

HimanshuMisra1 mentioned this pull request Nov 28, 2018

Resource Health RP changes for new resource type /events and /impacte… #4601

Merged

Comments

Conversation

HimanshuMisra1 commented Dec 9, 2017 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

olydis commented Dec 9, 2017

Uh oh!

olydis commented Dec 9, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olydis commented Dec 13, 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!

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!

olydis 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!

azuresdkciprbot commented Jan 10, 2018

Uh oh!

azuresdkciprbot commented Jan 10, 2018

Uh oh!

ravbhatnagar commented Jan 10, 2018

Uh oh!

olydis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olydis commented Jan 17, 2018

Uh oh!

olydis commented Jan 26, 2018

Uh oh!

AutorestCI commented Jan 26, 2018

Uh oh!

AutorestCI commented Jan 26, 2018

Uh oh!

AutorestCI commented Jan 26, 2018

Uh oh!

AutorestCI commented Jan 26, 2018

Uh oh!

Reviewers

Assignees

Labels

HimanshuMisra1 commented Dec 9, 2017 •

edited by azuresdkci

Loading