Skip to content

Conversation

@ericc1103
Copy link
Contributor

@ericc1103 ericc1103 commented Mar 14, 2018

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

@AutorestCI
Copy link

AutorestCI commented Mar 14, 2018

Automation for azure-sdk-for-python

Encountered a Subprocess error: (azure-sdk-for-python)

Command: ['/usr/local/bin/autorest', '/tmp/tmpllqcwpf2/rest/specification/applicationinsights/resource-manager/readme.md', '--multiapi', '--python', '--python-mode=update', '--python-sdks-folder=/tmp/tmpllqcwpf2/sdk', '[email protected]/[email protected]', '--version=preview']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4259; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/tmp/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4261)
   Loading AutoRest extension '@microsoft.azure/autorest.python' (2.1.40->2.1.40)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44->2.3.44)
ERROR: Referenced file 'file:///tmp/tmpllqcwpf2/rest/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/examples/WorkbookDelete.json' not found
    - file:///tmp/tmpllqcwpf2/rest/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/workbooks_API.json:122:12 ($.paths["/subscriptions/{subscriptionId}/resourceGroup/{resourceGroupName}/providers/microsoft.insights/workbooks/{resourceName}"].delete["x-ms-examples"].WorkbookList["$ref"])
FATAL: swagger-document/loader - FAILED
FATAL: Error: [OperationAbortedException] Error occurred. Exiting.
Process() cancelled due to exception : [OperationAbortedException] Error occurred. Exiting.

@AutorestCI
Copy link

AutorestCI commented Mar 14, 2018

Automation for azure-libraries-for-java

Encountered a Subprocess error: (azure-libraries-for-java)

Command: ['/usr/local/bin/autorest', '/tmp/tmp70ky3x9i/rest/specification/applicationinsights/resource-manager/readme.md', '--azure-libraries-for-java-folder=/tmp/tmp70ky3x9i/sdk', '--fluent', '--java', '--multiapi', '--verbose']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4259; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/tmp/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4259)
   Loading AutoRest extension '@microsoft.azure/autorest.java' (~2.1.32->2.1.49)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.38->2.3.38)
ERROR: Referenced file 'file:///tmp/tmp70ky3x9i/rest/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/examples/WorkbookDelete.json' not found
    - file:///tmp/tmp70ky3x9i/rest/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/workbooks_API.json:122:12 ($.paths["/subscriptions/{subscriptionId}/resourceGroup/{resourceGroupName}/providers/microsoft.insights/workbooks/{resourceName}"].delete["x-ms-examples"].WorkbookList["$ref"])
FATAL: swagger-document/loader - FAILED
FATAL: Error: [OperationAbortedException] Error occurred. Exiting.
Process() cancelled due to exception : [OperationAbortedException] Error occurred. Exiting.

@AutorestCI
Copy link

AutorestCI commented Mar 14, 2018

Automation for azure-sdk-for-go

Encountered a Subprocess error: (azure-sdk-for-go)

Command: ['/usr/local/bin/autorest', '/tmp/tmp_fec19om/rest/specification/applicationinsights/resource-manager/readme.md', '--go', '--go-sdk-folder=/tmp/tmp_fec19om/sdk', '--multiapi', '[email protected]/autorest.go@~2.1.95', '--use-onever', '--verbose']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4259; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/tmp/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4259)
   Loading AutoRest extension '@microsoft.azure/autorest.go' (~2.1.95->2.1.95)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.38->2.3.38)
Processing batch task - {"tag":"package-2015-05"} .
ERROR: Referenced file 'file:///tmp/tmp_fec19om/rest/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/examples/WorkbookDelete.json' not found
    - file:///tmp/tmp_fec19om/rest/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/workbooks_API.json:122:12 ($.paths["/subscriptions/{subscriptionId}/resourceGroup/{resourceGroupName}/providers/microsoft.insights/workbooks/{resourceName}"].delete["x-ms-examples"].WorkbookList["$ref"])
FATAL: swagger-document/loader - FAILED
FATAL: Error: [OperationAbortedException] Error occurred. Exiting.
Process() cancelled due to exception : [OperationAbortedException] Error occurred. Exiting.
Failure during batch task - {"tag":"package-2015-05"} -- false.

@AutorestCI
Copy link

AutorestCI commented Mar 14, 2018

Automation for azure-sdk-for-node

Encountered a Subprocess error: (azure-sdk-for-node)

Command: ['/usr/local/bin/autorest', '/tmp/tmpjownczrw/rest/specification/applicationinsights/resource-manager/readme.md', '--license-header=MICROSOFT_MIT_NO_VERSION', '--node-sdks-folder=/tmp/tmpjownczrw/sdk', '--nodejs']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4259; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/tmp/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4259)
   Loading AutoRest extension '@microsoft.azure/autorest.nodejs' (~2.1.25->2.1.32)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44->2.3.44)
ERROR: Referenced file 'file:///tmp/tmpjownczrw/rest/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/examples/WorkbookDelete.json' not found
    - file:///tmp/tmpjownczrw/rest/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/workbooks_API.json:122:12 ($.paths["/subscriptions/{subscriptionId}/resourceGroup/{resourceGroupName}/providers/microsoft.insights/workbooks/{resourceName}"].delete["x-ms-examples"].WorkbookList["$ref"])
FATAL: swagger-document/loader - FAILED
FATAL: Error: [OperationAbortedException] Error occurred. Exiting.
Process() cancelled due to exception : [OperationAbortedException] Error occurred. Exiting.

"sourceResourceId": "/subscriptions/00000000-0000-0000-0000-00000000/resourceGroups/MyGroup/providers/Microsoft.Web/sites/MyTestApp-CodeLens",
"workbook": {
"name": "Blah Blah Blah",
"serialziedData": "{\"MEDataModelRawJSON\":\"{\\n \\\"version\\\": \\\"1.4.1\\\",\\n \\\"isCustomDataModel\\\": true,\\n \\\"items\\\": [\\n {\\n \\\"id\\\": \\\"90a7134d-9a38-4c25-88d3-a495209873eb\\\",\\n \\\"chartType\\\": \\\"Area\\\",\\n \\\"chartHeight\\\": 4,\\n \\\"metrics\\\": [\\n {\\n \\\"id\\\": \\\"preview/requests/count\\\",\\n \\\"metricAggregation\\\": \\\"Sum\\\",\\n \\\"color\\\": \\\"msportalfx-bgcolor-d0\\\"\\n }\\n ],\\n \\\"priorPeriod\\\": false,\\n \\\"clickAction\\\": {\\n \\\"defaultBlade\\\": \\\"SearchBlade\\\"\\n },\\n \\\"horizontalBars\\\": true,\\n \\\"showOther\\\": true,\\n \\\"aggregation\\\": \\\"Sum\\\",\\n \\\"percentage\\\": false,\\n \\\"palette\\\": \\\"fail\\\",\\n \\\"yAxisOption\\\": 0,\\n \\\"title\\\": \\\"\\\"\\n },\\n {\\n \\\"id\\\": \\\"0c289098-88e8-4010-b212-546815cddf70\\\",\\n \\\"chartType\\\": \\\"Area\\\",\\n \\\"chartHeight\\\": 2,\\n \\\"metrics\\\": [\\n {\\n \\\"id\\\": \\\"preview/requests/duration\\\",\\n \\\"metricAggregation\\\": \\\"Avg\\\",\\n \\\"color\\\": \\\"msportalfx-bgcolor-j1\\\"\\n }\\n ],\\n \\\"priorPeriod\\\": false,\\n \\\"clickAction\\\": {\\n \\\"defaultBlade\\\": \\\"SearchBlade\\\"\\n },\\n \\\"horizontalBars\\\": true,\\n \\\"showOther\\\": true,\\n \\\"aggregation\\\": \\\"Avg\\\",\\n \\\"percentage\\\": false,\\n \\\"palette\\\": \\\"greenHues\\\",\\n \\\"yAxisOption\\\": 0,\\n \\\"title\\\": \\\"\\\"\\n },\\n {\\n \\\"id\\\": \\\"cbdaab6f-a808-4f71-aca5-b3976cbb7345\\\",\\n \\\"chartType\\\": \\\"Bar\\\",\\n \\\"chartHeight\\\": 4,\\n \\\"metrics\\\": [\\n {\\n \\\"id\\\": \\\"preview/requests/duration\\\",\\n \\\"metricAggregation\\\": \\\"Avg\\\",\\n \\\"color\\\": \\\"msportalfx-bgcolor-d0\\\"\\n }\\n ],\\n \\\"priorPeriod\\\": false,\\n \\\"clickAction\\\": {\\n \\\"defaultBlade\\\": \\\"SearchBlade\\\"\\n },\\n \\\"horizontalBars\\\": true,\\n \\\"showOther\\\": true,\\n \\\"aggregation\\\": \\\"Avg\\\",\\n \\\"percentage\\\": false,\\n \\\"palette\\\": \\\"magentaHues\\\",\\n \\\"yAxisOption\\\": 0,\\n \\\"title\\\": \\\"\\\"\\n },\\n {\\n \\\"id\\\": \\\"1d5a6a3a-9fa1-4099-9cf9-05eff72d1b02\\\",\\n \\\"grouping\\\": {\\n \\\"kind\\\": \\\"ByDimension\\\",\\n \\\"dimension\\\": \\\"context.application.version\\\"\\n },\\n \\\"chartType\\\": \\\"Grid\\\",\\n \\\"chartHeight\\\": 1,\\n \\\"metrics\\\": [\\n {\\n \\\"id\\\": \\\"basicException.count\\\",\\n \\\"metricAggregation\\\": \\\"Sum\\\",\\n \\\"color\\\": \\\"msportalfx-bgcolor-g0\\\"\\n },\\n {\\n \\\"id\\\": \\\"requestFailed.count\\\",\\n \\\"metricAggregation\\\": \\\"Sum\\\",\\n \\\"color\\\": \\\"msportalfx-bgcolor-f0s2\\\"\\n }\\n ],\\n \\\"priorPeriod\\\": true,\\n \\\"clickAction\\\": {\\n \\\"defaultBlade\\\": \\\"SearchBlade\\\"\\n },\\n \\\"horizontalBars\\\": true,\\n \\\"showOther\\\": true,\\n \\\"percentage\\\": false,\\n \\\"palette\\\": \\\"blueHues\\\",\\n \\\"yAxisOption\\\": 0,\\n \\\"title\\\": \\\"\\\"\\n }\\n ],\\n \\\"currentFilter\\\": {\\n \\\"eventTypes\\\": [\\n 1,\\n 2\\n ],\\n \\\"typeFacets\\\": {},\\n \\\"isPermissive\\\": false\\n },\\n \\\"timeContext\\\": {\\n \\\"durationMs\\\": 75600000,\\n \\\"endTime\\\": \\\"2018-01-31T20:30:00.000Z\\\",\\n \\\"createdTime\\\": \\\"2018-01-31T23:54:26.280Z\\\",\\n \\\"isInitialTime\\\": false,\\n \\\"grain\\\": 1,\\n \\\"useDashboardTimeRange\\\": false\\n },\\n \\\"jsonUri\\\": \\\"Favorite_BlankChart\\\",\\n \\\"timeSource\\\": 0\\n}\"}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is serializedData really misspelled?

do you actually need this full workbook content here? i'm not even sure where you got this as it looks to be a full serialized metric explorer content, NOT workbook content.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a sample data but I can put the actual workbook content.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just updated with the actual workbook content.

@marstr
Copy link
Member

marstr commented Mar 15, 2018

@AutorestCI rebuild azure-sdk-for-go

@Azure Azure deleted a comment from AutorestCI Mar 15, 2018
@Azure Azure deleted a comment from AutorestCI Mar 15, 2018
@Azure Azure deleted a comment from AutorestCI Mar 15, 2018
@Azure Azure deleted a comment from AutorestCI Mar 15, 2018
@ericc1103
Copy link
Contributor Author

ericc1103 commented Mar 15, 2018

@fearthecowboy Do we know why I am getting the following error when it build with MODE=python?

ERROR:swaggertosdk.autorest_tools:Command '['/usr/local/bin/autorest', '/git-restapi/specification/applicationinsights/resource-manager/readme.md', '--multiapi', '--python', '--python-mode=update', '--python-sdks-folder=/tmp/tmpm0i0y1tm/sdk', '[email protected]/autorest.python@~3.0', '--version=preview']' returned non-zero exit status 2.

@marstr
Copy link
Member

marstr commented Mar 15, 2018

@ericc1103 says:
@fearthecowboy Do we know why I am getting the following error when it build with MODE=python?

ERROR:swaggertosdk.autorest_tools:Command '['/usr/local/bin/autorest', '/git-restapi/specification/applicationinsights/resource-manager/readme.md', '--multiapi', '--python', '--python-mode=update', '--python-sdks-folder=/tmp/tmpm0i0y1tm/sdk', '[email protected]/autorest.python@~3.0', '--version=preview']' returned non-zero exit status 2.

@fearthecowboy is OOF sick today, and I happen to be here. If you look at the failure, here: https://travis-ci.org/Azure/azure-rest-api-specs/jobs/353635130#L680 it looks like you've got a parameter missing the "x-ms-parameter-location" property.

@lmazuel
Copy link
Member

lmazuel commented Mar 15, 2018

@ericc1103 Why is this PR pushing a Azure Monitor files where the title suggests it ApplicationInsights only?

@ericc1103 ericc1103 changed the title Creating new swagger entry for Workbook resource within applicationinsights Creating new swagger entry for Workbook resource Mar 15, 2018
@ericc1103
Copy link
Contributor Author

@fearthecowboy All checks are passed now. Are there any other things I would need to take an action?

@fearthecowboy
Copy link
Contributor

I'll take a look this afternoon

@fearthecowboy
Copy link
Contributor

Quick Question - @ericc1103 have these APIs been reviewed by ARM and the API Review Board?

@ericc1103
Copy link
Contributor Author

@fearthecowboy Not yet. Aren't they review this once Swagger is approved? I am a newbie on this type of process so let me know if I miss anything.

@fearthecowboy
Copy link
Contributor

@ericc1103 Ah, no, they'll have to approve it before it goes further. I suspect they'll have some feedback.

You should book a 1-hour skype meeting on Tuesday afternoon with Gaurav Bhatnagar;Vlad Joanovic;Marc Reyhner;David Justice;Darrel Miller;Garrett Serack;Niklas Gustafsson;Johan Stenberg; and we can go thru it.

@jhendrixMSFT
Copy link
Member

@AutorestCI regenerate azure-sdk-for-go

Copy link
Contributor

@ravbhatnagar ravbhatnagar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericc1103 please take a look at comments.

}
},
"paths": {
"/subscriptions/{subscriptionId}/resourceGroup/{resourceGroupName}/locations/{location}/providers/microsoft.insights/workbooks/category/{category}": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APi signature will not work. There is a segment missing after /workbooks. This segment is for the workbookName.

"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/Workbook"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should return a "value" array which will be a collection of workbooks. But anyway, as stated above the API signature does not indicate that. As per the API signature, customer is doing a GET on the category/{categoryName} and is expected to return a "category" entity

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix so that it will return object that contains an array. A workbook contains a category in properties field.

}
}
},
"/providers/microsoft.insights/workbooks/category/{category}": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the intent of this API? Can workbook/category live at tenant scope? They seem to be getting created under a resource group.

}
}
},
"/subscriptions/{subscriptionId}/resourceGroup/{resourceGroupName}/resourceName/{resourceName}/providers/microsoft.insights/workbooks": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the intent of this API to create a workbook at any resource scope? The workbookName segment is missing in the URL.

"put": {
"description": "Create a new workbook.",
"operationId": "Workbook_CreateLink",
"parameters": [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WHy does this not have a request/response body? The operation Id seems to indicate it is creating some link and not an actual workbook. What is this link?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are just create a link between a source resource and a workbook resource. Therefore, just return 200 is enough.

"type": "string",
"description": "This instance's version of the data model. This can change as new features are added that can be marked workbook."
},
"workbookId": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is the customer going to use this internal ID? Is it needed to surface it back to the user?

"type": "string",
"description": "Internally assigned unique id of the workbook definition."
},
"kind": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also a top level kind property. Can we call this something else?

"description": "Date and time in UTC of the last modification that was made to this workbook definition.",
"readOnly": true
},
"category": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different from kind? And what are the allowed values here?

"type": "string",
"description": "Workbook category, as defined by the user at creation time."
},
"tags": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tags is a top level property of a resource.

"type": "string",
"description": "Unique user id of the specific user that owns this workbook."
},
"sourceResourceId": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What id is this?

@ericc1103
Copy link
Contributor Author

I am going to abandon this PR due to the issue with a duplicated entries with a different casing after merging in my forked repo.

@ericc1103 ericc1103 closed this Mar 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants