[DO NOT MERGE, until feb 20] Adding 2018-01-01 version for metrics API #2435

LarryZhang19 · 2018-02-06T21:50:17Z

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.

msftclas · 2018-02-06T21:50:28Z

All CLA requirements met.

AutorestCI · 2018-02-06T21:51:42Z

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

AutorestCI · 2018-02-06T21:53:18Z

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

LarryZhang19 · 2018-02-06T21:56:59Z

I am seeing a error in one of the test, but it seems it failed in trying to install npm. And not related to my change.

LarryZhang19 · 2018-02-06T22:05:16Z

Please do not merge this PR until Feb 20th.

AutorestCI · 2018-02-06T23:11:47Z

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-06T23:13:12Z

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/monitor/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.

mcardosos · 2018-02-07T00:03:14Z

specification/monitor/resource-manager/readme.md

+tag: package-2018-02
+```
+### Tag: package-2018-02
+


Since there are both stable and preview swaggers, should this package be a preview?

What this package mean, is it the SDK? if so, it should remain preview.
Please do not merge after your signoff, since the GA date for the api is not til Feb 20th. That's when we would like the swagger spec to be published.

Yes, I have seen the publish date :) Thanks for adding this info from the very beginning, actually :D
The tag will become an SDK tag, yes, but if it includes preview APIs (I assume preview APIs might change anytime), the best would be to mark it as a preview package/tag.

In reply to: 166483608 [](ancestors = 166483608)

mcardosos · 2018-02-07T00:23:17Z

specification/monitor/resource-manager/microsoft.insights/stable/2018-01-01/metrics_API.json

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


examples [](start = 13, length = 8)

It is preferred that examples are only provided in the x-ms-examples section

mcardosos · 2018-02-07T00:32:29Z

...ion/monitor/resource-manager/microsoft.insights/stable/2018-01-01/metricDefinitions_API.json

+      "x-ms-parameter-location": "method",
+      "x-ms-skip-url-encoding": true
+    },
+	"MetricNamespaceParameter": {


MetricNamespaceParameter [](start = 2, length = 24)

Is it expected to generate this parameter as a method or a client parameter? (My guess is it is expected to be a method parameter)

If it is expected to be part of the method, please add "x-ms-parameter-location": "method"

In reply to: 166487297 [](ancestors = 166487297)

After consult with team member, will leave this as Client

mcardosos · 2018-02-07T00:35:49Z

specification/monitor/resource-manager/microsoft.insights/stable/2018-01-01/metrics_API.json

+          "type": "string",
+          "description": "The namespace of the metrics been queried"
+        },
+        "resourceregion": {


resourceregion [](start = 9, length = 14)

Should this be resourceRegion?

It should be resourceregion

mcardosos · 2018-02-07T00:39:21Z

...ion/monitor/resource-manager/microsoft.insights/stable/2018-01-01/metricDefinitions_API.json

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


definitions [](start = 3, length = 11)

Please add "type": "object" to the definitions that are type objects

Talked to colleague who helped publish the preview swagger and SDK, at this point, we should not change these types as previous version has already been published this way. And for the change I am making in this version, only include one optional parameter: metricnamespace, which should be string type.

mcardosos · 2018-02-07T00:44:10Z

specification/monitor/resource-manager/microsoft.insights/stable/2018-01-01/metrics_API.json

+    "TimeSeriesElement": {
+      "type": "object",
+      "properties": {
+        "metadatavalues": {


metadatavalues [](start = 9, length = 14)

metadataValues?

should be metadatavalues

mcardosos · 2018-02-07T00:47:54Z

specification/monitor/resource-manager/microsoft.insights/stable/2018-01-01/metrics_API.json

+        ],
+        "operationId": "Metrics_List",
+        "description": "**Lists the metric values for a resource**.",
+        "parameters": [


parameters [](start = 9, length = 10)

Since there are many parameters, x-ms-parameter-grouping could help gen better looking code

Same reason as the type one, we can not change this at this point as previous version is already using this style, changing it now will confuse existing customers

mcardosos · 2018-02-07T00:48:53Z

specification/monitor/resource-manager/microsoft.insights/stable/2018-01-01/metrics_API.json

+      "type": "string",
+      "description": "Client Api Version."
+    },
+	"MetricNamespaceParameter": {


MetricNamespaceParameter [](start = 2, length = 24)

Is this parameter meant for the client or the method?

AutorestCI · 2018-02-07T20:55:30Z

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-07T20:57:20Z

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/monitor/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.

mcardosos

Awesome! :D

mcardosos · 2018-02-08T22:32:58Z

Ping Larry on february 20

lmazuel · 2018-02-16T21:33:43Z

@AutorestCI rebuild azure-sdk-for-python

…, it's just a typo of the names.

AutorestCI · 2018-02-16T23:52:20Z

Automation for azure-sdk-for-go

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

Command: profileBuilder -s list -l ./profiles/2017-03-09/defintion.txt -name 2017-03-09
Finished with return code 127
and output:

/bin/sh: 1: profileBuilder: not found

lmazuel · 2018-02-17T00:10:10Z

@AutorestCI rebuild azure-sdk-for-python

mcardosos · 2018-02-17T00:58:08Z

@LarryZhang19
There are errors on model validation, PTAL

mcardosos · 2018-02-17T01:41:29Z

Still broken :(
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/342617554#L748

mcardosos

Everything green again :D

Adding 2018-01-01 version for metrics API

af0f444

azuresdkci assigned mcardosos Feb 6, 2018

AutorestCI mentioned this pull request Feb 6, 2018

[AutoPR monitor/resource-manager] Adding 2018-01-01 version for metrics API Azure/azure-sdk-for-python#1919

Closed

Fix extra comma at the end of line

fa60ee8

mcardosos reviewed Feb 7, 2018

View reviewed changes

Fix PR comments 1

2539273

mcardosos approved these changes Feb 8, 2018

View reviewed changes

mcardosos changed the title ~~Adding 2018-01-01 version for metrics API~~ [DO NOT MERGE, until feb 20] Adding 2018-01-01 version for metrics API Feb 8, 2018

AutorestCI mentioned this pull request Feb 16, 2018

[AutoPR monitor/resource-manager] [DO NOT MERGE, until feb 20] Adding 2018-01-01 version for metrics API Azure/azure-sdk-for-python#2007

Merged

Fixing top/orderby names, Please be reminded, there is no code change…

f6739fd

…, it's just a typo of the names.

Fix the sample to reflect the parameter

451731b

Fix meta data sample as well.

8ddf699

mcardosos approved these changes Feb 17, 2018

View reviewed changes

mcardosos merged commit d10a7a1 into Azure:master Feb 20, 2018

AlitzelMendez added a commit to AlitzelMendez/azure-rest-api-specs that referenced this pull request Apr 8, 2025

Validate typespec-azure Azure#2435

e25721c

[DO NOT MERGE, until feb 20] Adding 2018-01-01 version for metrics API #2435

[DO NOT MERGE, until feb 20] Adding 2018-01-01 version for metrics API #2435

Uh oh!

Conversation

LarryZhang19 commented Feb 6, 2018 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR information

Quality of Swagger

Uh oh!

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

Uh oh!

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!

LarryZhang19 commented Feb 6, 2018

Uh oh!

LarryZhang19 commented Feb 6, 2018

Uh oh!

AutorestCI commented Feb 6, 2018

Uh oh!

azuresdkciprbot commented Feb 6, 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!

mcardosos Feb 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

AutorestCI commented Feb 7, 2018

Uh oh!

azuresdkciprbot commented Feb 7, 2018

Uh oh!

mcardosos left a comment

Choose a reason for hiding this comment

Uh oh!

mcardosos commented Feb 8, 2018

Uh oh!

lmazuel commented Feb 16, 2018

Uh oh!

AutorestCI commented Feb 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-go

LarryZhang19 commented Feb 6, 2018 •

edited by azuresdkci

Loading

msftclas commented Feb 6, 2018 •

edited

Loading

AutorestCI commented Feb 6, 2018 •

edited

Loading

mcardosos Feb 7, 2018 •

edited

Loading

AutorestCI commented Feb 16, 2018 •

edited

Loading