Added GA API version for Scheduled Query Rule #2858

abversqr · 2018-04-12T12:22:38Z

Added GA API version for Scheduled Query Rule

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.

Added GA API version for Scheduled Query Rule

msftclas · 2018-04-12T12:22:50Z

All CLA requirements met.

AutorestCI · 2018-04-12T12:24:10Z

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#2460

AutorestCI · 2018-04-12T12:24:34Z

Automation for azure-sdk-for-node

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-node#2602

AutorestCI · 2018-04-12T12:24:34Z

Automation for azure-libraries-for-java

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

AutorestCI · 2018-04-12T12:26:01Z

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#1692

fearthecowboy · 2018-04-12T17:29:57Z

Yoink! @sarangan12 -- I'm assigning this to me so I can use it to train more reviewers. You get a freebie!

fearthecowboy · 2018-04-12T20:12:54Z

Hey @abversqr -- it appears you have a few validation errors in your spec -- can you correct these and update the PR?

https://travis-ci.org/Azure/azure-rest-api-specs/jobs/365583431#L736

If you want to run the linter yourself locally, install autorest and try:

# from the folder where the readme.md file is
autorest --azure-validator readme.md

It looks mostly like one typo (date=time?) missing examples and an API that should be defined.

fearthecowboy · 2018-04-12T20:15:01Z

Hey @abversqr -- this looks like a new API is being added -- has this been reviewed by the API review board?

If not, you should schedule a one hour SKYPE meeting with Garrett Serack, David Justice, Johan Stenberg, Gaurav Bhatnagar, Darrel Miller and Niklas Gustafsson . Preferably Tuesday afternoon sometime between 1 and 5pm.

fearthecowboy · 2018-04-12T20:16:40Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+  "info": {
+    "title": "Microsoft Insights API",
+    "version": "2018-04-16",
+    "x-ms-code-generation-settings": {


You shouldn't have x-ms-code-generation-settings in the actual swagger file (I don't think we even use this anymore at all).

fearthecowboy · 2018-04-12T20:22:26Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+        "tags": [
+          "scheduledQueryRules"
+        ],
+        "operationId": "createOrUpdateScheduledQueryRules",


Operation IDs should be GROUP_METHOD -- Where GROUP is a plural (ie, in this case ScheduledQueryRules and METHOD should be CreateOrUpdate which means that this one should be ScheduledQueryRules_CreateOrUpdate

Fixed in latest iteration

fearthecowboy

You will need to resolve a bunch of this before I can continue the review -- and the API review board should sign off.

fearthecowboy · 2018-04-12T20:25:03Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+  "paths": {
+    "/subscriptions/{subscriptionId}/resourcegroups/{resourceGroupName}/providers/microsoft.insights/scheduledQueryRules/{ruleName}": {
+      "put": {
+        "description": "Creates or updates an log search rule.\r\nRequest method: PUT\t\tRequest URI: https://management.azure.com/subscriptions/{subscription-id}/resourceGroups/{resource-group-name}/providers/microsoft.insights/scheduledQueryRules/{logsearch-rule-name}?api-version={api-version}",


The description should document the API (the presence of the swagger file is telling you how you should physically call this method). You can drop everything after Creates or updates an log search rule.

Fixed in latest iteration

fearthecowboy · 2018-04-12T20:27:47Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+            "schema": {
+              "$ref": "#/definitions/LogSearchRuleResource"
+            },
+            "examples": {


This should use x-ms-examples and the example data should be in a separate file. (and then the linter won't yell at you for missing examples :D )

Fixed in latest iteration

fearthecowboy · 2018-04-12T20:31:55Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+        "tags": [
+          "scheduledQueryRules"
+        ],
+        "operationId": "getScheduledQueryRule",


again, fix the operationId to match the pattern (ieScheduledQueryRules_Get)

fearthecowboy · 2018-04-12T20:32:18Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+            "schema": {
+              "$ref": "#/definitions/LogSearchRuleResource"
+            },
+            "examples": {


again, move example into external file with x-ms-examples

fearthecowboy · 2018-04-12T20:34:28Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+            "description": "The parameters of the rule to create or update."
+          }
+        ],
+        "responses": {


You should support a "default" response for errors. (see example https://github.com/abversqr/azure-rest-api-specs/blob/2e4c790d16eb6be9299db7a75798bca1697fd5e9/specification/monitor/resource-manager/microsoft.insights/stable/2018-03-01/metricAlert_API.json#L503 )

fearthecowboy · 2018-04-12T20:37:45Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+              }
+            }
+          },
+          "404": {


Error states should be modeled as "default" responses and have a model to deserialize the error into.

A lot of these things should have been caught in the API review board -- I'll pause the review here, and wait until after the review board has seen these APIs.

ravbhatnagar · 2018-04-17T05:27:35Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+          "readOnly": true,
+          "description": "Azure resource Id"
+        },
+        "name": {


this should also be readonly since the name comes from URL

Fixed in latest iteration

ravbhatnagar · 2018-04-17T05:29:02Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+            "modelAsString": true
+          }
+        },
+        "skuType": {


SKUs of a resource is indicated through the ARM top level "sku" property. The details can be found in the ARM resource provider contract

ravbhatnagar · 2018-04-17T05:29:26Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+          "format": "date-time",
+          "description": "Last time the rule was updated in IS08601 format."
+        },
+        "provisioningState": {


please define as an enum and list all the possible values.

Fixed in latest iteration

Resolving Comments

azuresdkciprbot · 2018-04-17T17:51:08Z

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

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

…rmat Couple of changes - 1. Made SKU as top level proprty as mentioned here - https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/resource-api-reference.md#put-resource 2. including generic error response format

azuresdkciprbot · 2018-04-18T18:18: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:

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

File: specification/monitor/resource-manager/readme.md
Before the PR: Warning(s): 37 Error(s): 0
After the PR: Warning(s): 37 Error(s): 1

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

ravbhatnagar · 2018-04-19T05:56:41Z

Looks good!

abversqr · 2018-04-19T11:12:38Z

Hey @fearthecowboy, I have resolved all comments. Can ou please have a look into this and sign-off if no blocking changes are pending,

fearthecowboy · 2018-04-19T21:18:41Z

@abversqr -- hey just a few things to clean up:

You've still got the examples in the swagger. if you have x-ms-examples, you should take out the inline examples.
It's still got 1 linter error and a few model validation errors:
see https://travis-ci.org/Azure/azure-rest-api-specs/jobs/368279045 -- you should have a PATCH operation for the resource, if you can't, you need to get an exemption from @ravbhatnagar .
see https://travis-ci.org/Azure/azure-rest-api-specs/jobs/368279046

You can run the model validator locally if you install it :

npm install -g oav

And then run it from your azure-rest-api-specs\specification\monitor\resource-manager folder:

oav validate-example microsoft.insights\stable\2018-04-16\scheduledQueryRule_API.json

Which will tell you what's not quite right with your examples.

Thanks!

1. Removed SKU - Billing Model is still under discussion, hence need not to be exposed to customers. 2. Enabled field ichanged to align with camel case. 3. 'throttleTillDate' in properties.action changed to 'throttlingInMin' to denote correct meaning and it’s data type changed to number instead of date. 4. 'status' to be removed in properties.action from examples. 5. 'severity' made a required field in properties.action. 6. Removed Examples from original spec json

azuresdkciprbot · 2018-04-23T16:49:18Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(17 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-23T16:49:18Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(20 total)

❌1 new Errors.(1 total)

Code	Id	Source	Message
TrackedResourcePatchOperation	R3026	Link	Tracked resource 'LogSearchRuleResource' must have patch operation that at least supports the update of tags. It's strongly recommended that the PATCH operation supports update of all mutable properties as well.

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

Updated Example to reflect AI instances in tags and 'dataSourceId"

azuresdkciprbot · 2018-04-23T17:11:29Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(17 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-23T17:11:29Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(20 total)

❌1 new Errors.(1 total)

Code	Id	Source	Message
TrackedResourcePatchOperation	R3026	Link	Tracked resource 'LogSearchRuleResource' must have patch operation that at least supports the update of tags. It's strongly recommended that the PATCH operation supports update of all mutable properties as well.

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-23T17:42:42Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-23T17:42:42Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(0 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

Restore Tags Description

azuresdkciprbot · 2018-04-23T18:11:30Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(20 total)

❌1 new Errors.(1 total)

Code	Id	Source	Message
TrackedResourcePatchOperation	R3026	Link	Tracked resource 'LogSearchRuleResource' must have patch operation that at least supports the update of tags. It's strongly recommended that the PATCH operation supports update of all mutable properties as well.

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-23T18:11:30Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(17 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

Fixed errores reported by running command "oav validate-example"

fearthecowboy · 2018-04-24T17:28:30Z

...on/monitor/resource-manager/microsoft.insights/stable/2018-04-16/scheduledQueryRule_API.json

+            "$ref": "#/parameters/SubscriptionIdParameter"
+          }
+        ],
+        "responses": {


You should have a default response modeled here -- if the service fails for any reason, the client needs to be able to get the error response.

azuresdkciprbot · 2018-04-24T17:29:07Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(20 total)

❌1 new Errors.(1 total)

Code	Id	Source	Message
TrackedResourcePatchOperation	R3026	Link	Tracked resource 'LogSearchRuleResource' must have patch operation that at least supports the update of tags. It's strongly recommended that the PATCH operation supports update of all mutable properties as well.

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-24T17:29:07Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(17 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

Changes - 1. Added Default Response payload in all APIs 2. Made azNs description more clear

fearthecowboy

LGTM.

There is still a R3026 Linter violation -- the RP will fix in a subsequent PR : "we are working on this, but will take couple of weeks to get this done. Once this done, we will push this change in new PR for review. Let us know if you see any concern with this."

azuresdkciprbot · 2018-04-24T17:53:23Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(20 total)

❌1 new Errors.(1 total)

Code	Id	Source	Message
TrackedResourcePatchOperation	R3026	Link	Tracked resource 'LogSearchRuleResource' must have patch operation that at least supports the update of tags. It's strongly recommended that the PATCH operation supports update of all mutable properties as well.

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-24T17:53:23Z

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/monitor/resource-manager/readme.md

⚠️

0 new Warnings.(17 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

Added GA API version for Scheduled Query Rule

2e4c790

Added GA API version for Scheduled Query Rule

azuresdkci assigned sarangan12 Apr 12, 2018

azuresdkci requested a review from sarangan12 April 12, 2018 12:23

AutorestCI mentioned this pull request Apr 12, 2018

[AutoPR monitor/resource-manager] Added GA API version for Scheduled Query Rule Azure/azure-sdk-for-python#2386

Closed

This was referenced Apr 12, 2018

[AutoPR monitor/resource-manager] Added GA API version for Scheduled Query Rule Azure/azure-sdk-for-node#2721

Merged

[AutoPR monitor/resource-manager] Added GA API version for Scheduled Query Rule AutorestCI/azure-libraries-for-java#71

Merged

AutorestCI mentioned this pull request Apr 12, 2018

[AutoPR monitor/resource-manager] Added GA API version for Scheduled Query Rule Azure/azure-sdk-for-go#1564

Closed

fearthecowboy assigned fearthecowboy and unassigned sarangan12 Apr 12, 2018

fearthecowboy reviewed Apr 12, 2018

View reviewed changes

fearthecowboy suggested changes Apr 12, 2018

View reviewed changes

fearthecowboy added the Changes Required label Apr 12, 2018

ravbhatnagar reviewed Apr 17, 2018

View reviewed changes

ravbhatnagar added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 17, 2018

Resolving Comments

d183bad

Resolving Comments

AutorestCI mentioned this pull request Apr 17, 2018

[AutoPR monitor/resource-manager] Added GA API version for Scheduled Query Rule Azure/azure-sdk-for-python#2416

Merged

ravbhatnagar added the ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review label Apr 19, 2018

ravbhatnagar removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 19, 2018

Updated Example to reflect AI instances in tags and 'dataSourceId"

c39fa0c

Updated Example to reflect AI instances in tags and 'dataSourceId"

made 'tags' details more descriptive

5762ef5

AutorestCI mentioned this pull request Apr 23, 2018

[AutoPR monitor/resource-manager] Added GA API version for Scheduled Query Rule Azure/azure-sdk-for-go#1676

Merged

Restore Tags Description

caabc1b

Restore Tags Description

Fixed Exmaple Validation

ef75786

Fixed errores reported by running command "oav validate-example"

fearthecowboy reviewed Apr 24, 2018

View reviewed changes

Added Default Response payload in all APIs

fb180fb

Changes - 1. Added Default Response payload in all APIs 2. Made azNs description more clear

fearthecowboy approved these changes Apr 24, 2018

View reviewed changes

fearthecowboy merged commit 63892f9 into Azure:master Apr 24, 2018

Klaas- mentioned this pull request Jul 24, 2025

Scheduled query rules API 2021-08-01 missing Azure/azure-sdk-for-python#42181

Closed

Added GA API version for Scheduled Query Rule #2858

Added GA API version for Scheduled Query Rule #2858

Uh oh!

Conversation

abversqr commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR information

Quality of Swagger

Uh oh!

msftclas commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AutorestCI commented Apr 12, 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 Apr 12, 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 Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-libraries-for-java

Uh oh!

AutorestCI commented Apr 12, 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!

fearthecowboy commented Apr 12, 2018

Uh oh!

fearthecowboy commented Apr 12, 2018

Uh oh!

fearthecowboy commented Apr 12, 2018

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!

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

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 Apr 17, 2018

Uh oh!

azuresdkciprbot commented Apr 18, 2018

Uh oh!

ravbhatnagar commented Apr 19, 2018

Uh oh!

abversqr commented Apr 19, 2018

Uh oh!

fearthecowboy commented Apr 19, 2018

abversqr commented Apr 12, 2018 •

edited

Loading

msftclas commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading