Adding BillingMeters API by andreyse · Pull Request #2745 · Azure/azure-rest-api-specs

andreyse · 2018-03-23T19:03:02Z

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.

AutorestCI · 2018-03-23T19:05:36Z

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

dsgouda · 2018-03-23T19:12:20Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/Global.json

+    "application/json"
+  ],
+  "paths": {
+    "/subscriptions/{subscriptionId}/providers/Microsoft.Web/billingMeters": {


This should be referenced in the readme.md config file here

Thanks for review. I added it there

AutorestCI · 2018-03-23T19:15:16Z

Automation for azure-libraries-for-java

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

AutorestCI · 2018-03-23T19:28:06Z

Automation for azure-sdk-for-node

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

AutorestCI · 2018-03-23T21:05:22Z

Automation for azure-sdk-for-go

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

dsgouda

Left a few more comments.
Please check the linter violations too

dsgouda · 2018-03-23T21:03:06Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/Global.json

+        ],
+        "summary": "Gets a list of meters for a given location.",
+        "description": "Gets a list of meters for a given location.",
+        "operationId": "Global_GetBillingMeters",


Recommended operation id is GlobalBillingMeters_get or BillingMeters_Get

Please add x-ms-examples for this operation

dsgouda · 2018-03-23T21:03:48Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/Global.json

+        "responses": {
+          "200": {
+            "description": "OK",
+            "schema": {


inline definitions are discouraged. Please create a model definition for this array

Also, please add the x-ms-pageable extension here.

dsgouda · 2018-03-23T21:06:41Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/Global.json

+  },
+  "definitions": {
+    "BillingMeter": {
+      "description": "Billing meter.",


please provide a more useful description, applies to other properties too

andreyse · 2018-03-24T03:56:40Z

Look like I added billingmeters in the wrong file. Putting everything together now and addressing your comments

…pecs

andreyse · 2018-03-24T04:24:17Z

Added example

dsgouda

Left some comments.

dsgouda · 2018-03-26T20:16:27Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/ResourceProvider.json

        }
      }
+    },
+	"/subscriptions/{subscriptionId}/providers/Microsoft.Web/billingMeters": {


nit: fix indentation

can you please provide more details on this?

The indentation is off to the right by a tab, just align it to the closing bracket above

dsgouda · 2018-03-26T20:17:50Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/ResourceProvider.json

+        }
+      ],
+      "properties": {
+        "properties": {


Consider creating another model for these properties, something like BillingMeterProperties and then apply the x-ms-client-flatten extension

dsgouda · 2018-03-26T20:19:38Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/ResourceProvider.json

+            }
+          },
+          "default": {
+            "description": "Default error response",


Consider adding a better description, an example here

I removed this. Other our API in this json don't have it .

dsgouda · 2018-03-26T20:20:49Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/ResourceProvider.json

+      "type": "object",
+      "properties": {
+        "value": {
+          "description": "Collection of resources.",


Please verify description or add more relevant information

dsgouda · 2018-03-26T20:22:01Z

...ication/web/resource-manager/Microsoft.Web/stable/2016-03-01/examples/ListBillingMeters.json

+            }
+          },
+          {
+            "id": "subscriptions/34adfa4f-cedf-4dc0-ba29-b6d1a69ab345/providers/Microsoft.Web/billingMeters/6318825b-98b6-4b3c-883f-52a9d1560dae",


It is discouraged to use production style subscription ids in public repo, please replace these with {subId}, {billingId}, etc

dsgouda · 2018-03-26T20:22:31Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/ResourceProvider.json

+            "name": "billingLocation",
+            "in": "query",
+            "description": "Azure Location of billable resource",
+            "type": "string"


Is this parameter required?

No, it is not required. You can call api without billingLocation , it will return default values.

dsgouda · 2018-03-26T20:22:58Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/ResourceProvider.json

+      "get": {
+        "summary": "Gets a list of meters for a given location.",
+        "description": "Gets a list of meters for a given location.",
+        "operationId": "ListBillingMeters",


Recommended pattern is BillingMeters_List

dsgouda · 2018-03-26T20:26:58Z

specification/web/resource-manager/Microsoft.Web/stable/2016-03-01/ResourceProvider.json

+          "default": {
+            "description": "Default error response",
+            "schema": {
+              "$ref": "./CommonDefinitions.json#/definitions/ErrorResponse"


ErrorResponse reference does not exist, please fix this

…pecs

dsgouda

LGTM

Adding BillingMeters API

3466517

azuresdkci assigned dsgouda Mar 23, 2018

AutorestCI mentioned this pull request Mar 23, 2018

[AutoPR web/resource-manager] Adding BillingMeters API Azure/azure-sdk-for-python#2271

Merged

dsgouda reviewed Mar 23, 2018

View reviewed changes

AutorestCI mentioned this pull request Mar 23, 2018

[AutoPR web/resource-manager] Adding BillingMeters API AutorestCI/azure-libraries-for-java#3

Merged

AutorestCI mentioned this pull request Mar 23, 2018

[AutoPR web/resource-manager] Adding BillingMeters API Azure/azure-sdk-for-node#2545

Merged

Adding reference to readme.md

2c9ced7

AutorestCI mentioned this pull request Mar 23, 2018

[AutoPR web/resource-manager] Adding BillingMeters API Azure/azure-sdk-for-go#1423

Merged

dsgouda suggested changes Mar 23, 2018

View reviewed changes

andreyse and others added 2 commits March 23, 2018 20:51

"Added billingmeters in the correct file"

8240f54

Update readme.md

2023907

andreyse added 2 commits March 23, 2018 21:19

Adding example for ListBillingMeters

c36bac6

Merge branch 'master' of https://github.com/andreyse/azure-rest-api-s…

77a4161

…pecs

dsgouda suggested changes Mar 26, 2018

View reviewed changes

dsgouda reviewed Mar 26, 2018

View reviewed changes

andreyse added 3 commits March 29, 2018 15:04

Update ResourceProvider.json

7b6a81c

Addressing comments

43630ab

Merge branch 'master' of https://github.com/andreyse/azure-rest-api-s…

1723dbe

…pecs

dsgouda approved these changes Mar 30, 2018

View reviewed changes

dsgouda merged commit 47d267f into Azure:master Mar 30, 2018

Conversation

andreyse commented Mar 23, 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!

AutorestCI commented Mar 23, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AutorestCI commented Mar 23, 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 Mar 23, 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 Mar 23, 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!

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

andreyse commented Mar 24, 2018

Uh oh!

andreyse commented Mar 24, 2018

Uh oh!

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

dsgouda left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

andreyse commented Mar 23, 2018 •

edited by azuresdkci

Loading

AutorestCI commented Mar 23, 2018 •

edited

Loading

AutorestCI commented Mar 23, 2018 •

edited

Loading

AutorestCI commented Mar 23, 2018 •

edited

Loading

AutorestCI commented Mar 23, 2018 •

edited

Loading