Add swagger for annotation and work item config api for microsoft.insights components namespace for api version 2015-05-01 #2384

jpiyali · 2018-01-31T18:59:14Z

…space for api version 2015-05-01

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.

…space for api version 2015-05-01

AutorestCI · 2018-01-31T19:00:19Z

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

AutorestCI · 2018-01-31T19:00:23Z

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

lmazuel · 2018-01-31T22:32:18Z

Tagging @ravbhatnagar since new file in "stable" folder

lmazuel · 2018-01-31T22:35:15Z

Please fix all PR_only=true CI issue
Please plug this file in the Readme

AutorestCI · 2018-02-06T00:14:32Z

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#2022

AutorestCI · 2018-02-06T22:21:56Z

Swagger to SDK encountered a Subprocess error: (Azure/azure-sdk-for-go)

Command: profileBuilder -s preview -name preview
Finished with return code 127
and output:

/bin/sh: 1: profileBuilder: not found

jpiyali · 2018-02-06T22:33:56Z

@ravbhatnagar @lmazuel - all checks passed for PR=true except live validation. Can you please review?

jpiyali · 2018-02-07T00:28:47Z

Thank you @lmazuel , this was my first PR so had a few commits before getting rid of all errors. Do you recommend a separate PR for my next API (they are ready), or should I tag onto this one?

lmazuel · 2018-02-07T00:38:38Z

I personally like having atomic PR, but if this is a new API in the same Readme, you may use this same PR to simplify future ARM review.

AutorestCI · 2018-02-07T01:00:10Z

Swagger to SDK encountered a Subprocess error: (Azure/azure-sdk-for-go)

Command: profileBuilder -s preview -name preview
Finished with return code 127
and output:

/bin/sh: 1: profileBuilder: not found

azuresdkciprbot · 2018-02-07T01:00:43Z

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

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

AutorestCI · 2018-02-12T23:48:18Z

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#1119

azuresdkciprbot · 2018-02-12T23:52:35Z

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

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

jpiyali · 2018-02-12T23:55:20Z

@lmazuel - I've taken care of PR=true issues for the new api as well. Please take a look. Also, can you please help get some attention for the ARMFeedback tag?

ravbhatnagar · 2018-02-15T21:18:09Z

...insights/resource-manager/microsoft.insights/stable/2015-05-01/componentAnnotations_API.json

+              }
+			}
+          },
+          "409": {


remove all 4xx related errors and include a "default" model which captures all error responses. Check Batch api spec in this repo. They have a CloudError model.

Updated this just now in both files.

ravbhatnagar · 2018-02-15T21:19:15Z

...insights/resource-manager/microsoft.insights/stable/2015-05-01/componentAnnotations_API.json

+      "type": "object",
+      "description": "Annotation associated with an application insights resource.",
+      "properties": {
+        "AnnotationName": {


all RP specific properties should go inside the "properties" property. Top level allowed properties are id, name, type, tags, location etc.

So properties like category, eventtime etc need to be moved.

This is a released version in PROD for 2+ years now. We'll update this in next api-version.

Ok, since this is existing behavior which will break clients, lets identify the path forward in the next major version update.

ravbhatnagar · 2018-02-15T21:19:53Z

...insights/resource-manager/microsoft.insights/stable/2015-05-01/componentAnnotations_API.json

+          "format": "date-time",
+          "description": "Time when event occurred"
+        },
+        "Id": {


please make sure all property names are camel cased.

This is a released version in PROD for 2+ years now. We'll update this in next api-version.

Yes, please.

ravbhatnagar · 2018-02-15T21:20:30Z

...ghts/resource-manager/microsoft.insights/stable/2015-05-01/componentWorkItemConfigs_API.json

+          "nextLinkName": null
+        }
+      },
+      "post": {


put for create. Why is this a POST.

Its a POST because the Url path doesn't uniquely define the unique resource. The url path is /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/microsoft.insights/components/{resourceName}/WorkItemConfigs and there can be multiple for single resource.

POST should not be used for creating resources. Its not idempotent. It breaks consistency around how resources get provisioned in across Azure. Moreover, although its possible to do it via POST, we strongly recommend teams against creating multiple resources in one API call.

This api does not create multiple resources in one API call, it creates a single one only. I agree that it is not idempotent, but did not know this as a requirement. Will keep this guideline for future api versions. Where are api guidelines documented for ARM?

ravbhatnagar · 2018-02-15T21:21:34Z

...ghts/resource-manager/microsoft.insights/stable/2015-05-01/componentWorkItemConfigs_API.json

+        }
+      }
+    },
+    "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/microsoft.insights/components/{resourceName}/DefaultWorkItemConfig": {


Why is the defaultWorkitemConfig part of this URL instead of a parameter as is the case in other APIs like DELETE, PUT etc.

agreed, we'll fix this in our next release version.

I dont understand. Is this just a swagger fix or a service side fix? I was hoping this is just an issue with how its described in swagger.

This is the way the API was published. Essentially to retrieve default configuration for a resource. This is not a swagger fix, this is the swagger documentation for apis that are in PROD today. We're documenting our current public apis that are in production.

ravbhatnagar · 2018-02-15T21:22:03Z

...ghts/resource-manager/microsoft.insights/stable/2015-05-01/componentWorkItemConfigs_API.json

+    }
+  },
+  "definitions": {
+    "WorkItemConfiguration": {


Same comment about top level properties that are allowed.

agreed, will change this in our next public version.

ravbhatnagar · 2018-02-15T21:22:59Z

@jpiyali - some comments from ARM side. Please take a look.

…or definition

jpiyali · 2018-02-15T22:40:26Z

@ravbhatnagar - thank you for your review comments, I've responded and updated.

ravbhatnagar · 2018-02-21T01:58:13Z

Since these are existing APIs in prod for several years, approving the PR. Fixing these will most likely mean breaking changes in the client. Email conversation ongoing with the AI team.
SIgning off on this PR.

jpiyali · 2018-02-21T19:38:08Z

@lmazuel - can you please help merge the PR?

lmazuel · 2018-02-21T19:39:59Z

@AutorestCI rebuild azure-sdk-for-python

Add swagger for annotation api for microsoft.insights components name…

4e685d3

…space for api version 2015-05-01

azuresdkci assigned lmazuel Jan 31, 2018

lmazuel added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Jan 31, 2018

lmazuel requested a review from ravbhatnagar January 31, 2018 22:31

jpiyali added 3 commits February 5, 2018 11:18

Merge remote-tracking branch 'upstream/master'

724d58a

Address pr comment to include file in readme

1336db6

Fix issues with PR=true

e41b658

AutorestCI mentioned this pull request Feb 6, 2018

[AutoPR applicationinsights/resource-manager] Add swagger for annotation api for microsoft.insights components namespace for api version 2015-05-01 Azure/azure-sdk-for-python#1907

Closed

jpiyali added 4 commits February 5, 2018 16:27

Fix more PR=true errors

74015ef

fix more errors

3c9eb16

Fix more PR=true errors

d65c3df

Fix syntax error

109cec0

Azure deleted a comment from AutorestCI Feb 6, 2018

jpiyali added 6 commits February 6, 2018 11:44

Fix more errors

62334ee

Fix syntax

872b444

fix 2 more Pr=true errors

beb1e96

Try to fix errors again

f7c2334

Fix syntax

a39d1a0

Fix more errors

c284f75

Add work item configuration api

e89fc31

jpiyali changed the title ~~Add swagger for annotation api for microsoft.insights components namespace for api version 2015-05-01~~ Add swagger for annotation and work item config api for microsoft.insights components namespace for api version 2015-05-01 Feb 7, 2018

Fix pr=true error

d73b422

Azure deleted a comment from AutorestCI Feb 13, 2018

ravbhatnagar reviewed Feb 15, 2018

View reviewed changes

Address code review comment for replacing error codes with defaultErr…

a07c46b

…or definition

AutorestCI mentioned this pull request Feb 15, 2018

[AutoPR applicationinsights/resource-manager] Add swagger for annotation and work item config api for microsoft.insights components namespace for api version 2015-05-01 Azure/azure-sdk-for-python#1996

Merged

Remove error codes from examples

ae8994b

ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review Conditionally-Merged and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Feb 21, 2018

lmazuel merged commit 1d7d3bf into Azure:master Feb 21, 2018

AutorestCI mentioned this pull request Feb 21, 2018

[AutoPR applicationinsights/resource-manager] Add swagger for annotation and work item config api for microsoft.insights components namespace for api version 2015-05-01 Azure/azure-sdk-for-go#1118

Closed

Add swagger for annotation and work item config api for microsoft.insights components namespace for api version 2015-05-01 #2384

Add swagger for annotation and work item config api for microsoft.insights components namespace for api version 2015-05-01 #2384

Uh oh!

Conversation

jpiyali commented Jan 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR information

Quality of Swagger

Uh oh!

AutorestCI commented Jan 31, 2018

Uh oh!

AutorestCI commented Jan 31, 2018

Uh oh!

lmazuel commented Jan 31, 2018

Uh oh!

lmazuel commented Jan 31, 2018

Uh oh!

AutorestCI commented Feb 6, 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 Feb 6, 2018

Uh oh!

jpiyali commented Feb 6, 2018

Uh oh!

jpiyali commented Feb 7, 2018

Uh oh!

lmazuel commented Feb 7, 2018

Uh oh!

AutorestCI commented Feb 7, 2018

Uh oh!

azuresdkciprbot commented Feb 7, 2018

Uh oh!

AutorestCI commented Feb 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!

azuresdkciprbot commented Feb 12, 2018

Uh oh!

jpiyali commented Feb 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!

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!

Choose a reason for hiding this comment

Uh oh!

ravbhatnagar commented Feb 15, 2018

jpiyali commented Jan 31, 2018 •

edited

Loading

AutorestCI commented Feb 6, 2018 •

edited

Loading

AutorestCI commented Feb 12, 2018 •

edited

Loading