Added schema spec for cost allocation tags #2742

asarkar84 · 2018-03-23T01:52:24Z

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 Tags filter for budgets and updated the api version

Updated comments

Incorporated review comments

Incorporated review comment

Added Tags filter and grouping for UsageDetails

Added reservation recommendations and tags

Incorporated review comments

Removed unwanted space

Removed extra whitespace

Added schema spec for cost allocation tags

AutorestCI · 2018-03-23T01:54:23Z

Automation for azure-sdk-for-python

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

AutorestCI · 2018-03-23T01:56:11Z

Automation for azure-libraries-for-java

Nothing to generate for azure-libraries-for-java

AutorestCI · 2018-03-23T01:59:43Z

Automation for azure-sdk-for-node

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

AutorestCI · 2018-03-23T02:01:14Z

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

amarzavery · 2018-03-23T02:27:13Z

...cation/consumption/resource-manager/Microsoft.Consumption/stable/2018-03-31/consumption.json

+      "in": "path",
+      "description": "Azure Billing Account ID.",
+      "required": true,
+      "type": "string"


This causes a breaking change in the sdk. Refer to this comment.
This parameter should have "x-ms-parameter-location": "method" extension applied.

Whoever merged the PR last time did not catch that grainParameter defined below also has missing"x-ms-parameter-location": "method" extension

This should have been caught when this PR was sent 9 days ago.

@marstr - Please take a look.

Added "x-ms-parameter-location": "method" for grainParameter.
billing accounts parameter requires some other changes from our end. Will update that shortly.

Sure. But please make sure to add "x-ms-parameter-location": "method" for "billingAccountIdParameter" as well.

Updated Get Operation

Incorporated review comments

sergey-shandar · 2018-03-23T20:09:58Z

@amarzavery Could you confirm that the issue is fixed?

sergey-shandar · 2018-03-23T20:10:24Z

@ravbhatnagar new operations.

amarzavery · 2018-03-23T21:58:49Z

@sergey-shandar - The issue has been fixed for grainParameter but not for "billingAccountIdParameter". That should also have "x-ms-parameter-location": "method". Please make sure to not merge the PR until that happens.

sergey-shandar · 2018-03-26T18:38:36Z

@amarzavery thank you. @asarkar84 could you fix billingAccountIdParameter as well?

Review comments incorporated

asarkar84 · 2018-03-26T22:04:25Z

billingAccountIdParameter is fixed as well.

Updated path

amarzavery · 2018-03-27T00:57:18Z

...on/resource-manager/Microsoft.Consumption/stable/2018-03-31/examples/CostAllocationTags.json

+        "id": "providers/Microsoft.CostManagement/billingAccounts/{billingaccount-id}/providers/Microsoft.Consumption/costAllocationTags/costAllocationTags1",
+        "name": "costAllocationTags1",
+        "type": "Microsoft.Consumption/costAllocationTags",
+        "eTag": "\"1d34d012214157f\"",


@asarkar84 - As per the definition of "eTag" in the spec, it is a string.

What is the reason behind escaping the quotes "\"1d34d012214157f\""? Wouldn't this "1d34d012214157f" be sufficient? Why is the service sending a string with quotes in it ?
/cc @sergey-shandar - FYI.

This is how azure services or apis do eTag: " For example, I checked the CosmosDB data explorer which is publicly available. I found _eTag property exposed there and the service returns it with quotes. Storage also has quotes in their eTag. Since 2 double quotes are not accepted in json we need to escape one which is inside the value.

PFB the CosmosDB example for reference:
_etag": ""00000e3a-0000-0000-0000-59c94cd20000"",

@amarzavery Just refer any public API for reference, eTag will have quote. Escape char is used as per json standard since it will not accept 2 quotes like this ""

Thank you for the explanation. That makes sense.

asarkar84 · 2018-03-27T01:45:23Z

@amarzavery @sergey-shandar Just wanted to know for my own understanding, what is the use of this: "x-ms-parameter-location": "method"

why this is not required for apiVersionParameter and subscriptionIdParameter? It will be good for me to know this in future.

amarzavery · 2018-03-27T02:01:08Z

@asarkar8 - You can find detailed explanation about the extension "x-ms-parameter-location": "method" over here.

asarkar84 · 2018-03-27T02:04:03Z

I appreciate! This is helpful.

asarkar84 · 2018-04-05T20:05:36Z

The path has been finalized at our end. Please resume the review and merge the PR if it looks good.

ravbhatnagar · 2018-04-05T23:40:54Z

Signing off from ARM side.

asarkar84 and others added 18 commits March 8, 2018 15:44

Added Tags filter for budgets and updated the api version

7b927cf

Added Tags filter for budgets and updated the api version

Updated comments

c641158

Updated comments

Incorporated review comments

0e276b2

Incorporated review comments

Incorporated review comments

7be4364

Incorporated review comments

Incorporated review comment

bff7aa7

Incorporated review comment

Merge remote-tracking branch 'upstream/master'

011c9f0

Added Tags filter and grouping for UsageDetails

002a6db

Added Tags filter and grouping for UsageDetails

Merge remote-tracking branch 'upstream/master'

020911c

Added reservation recommendations and tags

502d70e

Added reservation recommendations and tags

Merge remote-tracking branch 'upstream/master'

b41bd12

Incorporated review comments

7956270

Incorporated review comments

Incorporated review comments

346ce83

Incorporated review comments

Removed unwanted space

a0072f9

Removed unwanted space

Removed extra whitespace

ee9bfc3

Removed extra whitespace

add swagger->sdk config for go

6df7a9e

fix broken tags for go

7a5eab1

Merge remote-tracking branch 'upstream/master'

623ff00

Added schema spec for cost allocation tags

6388784

Added schema spec for cost allocation tags

azuresdkci assigned sergey-shandar Mar 23, 2018

AutorestCI mentioned this pull request Mar 23, 2018

[AutoPR consumption/resource-manager] Added schema spec for cost allocation tags Azure/azure-sdk-for-python#2263

Merged

Merge remote-tracking branch 'upstream/master'

a8d47d1

AutorestCI mentioned this pull request Mar 23, 2018

[AutoPR consumption/resource-manager] Added schema spec for cost allocation tags Azure/azure-sdk-for-node#2536

Merged

AutorestCI mentioned this pull request Mar 23, 2018

[AutoPR consumption/resource-manager] Added schema spec for cost allocation tags Azure/azure-sdk-for-go#1407

Closed

amarzavery reviewed Mar 23, 2018

View reviewed changes

asarkar84 added 2 commits March 22, 2018 19:48

Updated Get Operation

3ccf1b4

Updated Get Operation

Incorporated review comments

884aabe

Incorporated review comments

sergey-shandar added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Mar 23, 2018

Review comments incorporated

3d1c7c1

Review comments incorporated

Updated path

def6325

Updated path

amarzavery reviewed Mar 27, 2018

View reviewed changes

ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Apr 5, 2018

sergey-shandar approved these changes Apr 6, 2018

View reviewed changes

sergey-shandar merged commit 895ab21 into Azure:master Apr 6, 2018

AutorestCI mentioned this pull request Apr 6, 2018

[AutoPR consumption/resource-manager] Added schema spec for cost allocation tags Azure/azure-sdk-for-go#1525

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added schema spec for cost allocation tags #2742

Added schema spec for cost allocation tags #2742

asarkar84 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

AutorestCI commented Mar 23, 2018 •

edited

Loading

amarzavery Mar 23, 2018

amarzavery Mar 23, 2018

amarzavery Mar 23, 2018

asarkar84 Mar 23, 2018

amarzavery Mar 23, 2018

sergey-shandar commented Mar 23, 2018

sergey-shandar commented Mar 23, 2018

amarzavery commented Mar 23, 2018

sergey-shandar commented Mar 26, 2018

asarkar84 commented Mar 26, 2018

amarzavery Mar 27, 2018

asarkar84 Mar 27, 2018 •

edited

Loading

asarkar84 Mar 27, 2018

amarzavery Mar 27, 2018

asarkar84 commented Mar 27, 2018

amarzavery commented Mar 27, 2018

asarkar84 commented Mar 27, 2018

asarkar84 commented Apr 5, 2018

ravbhatnagar commented Apr 5, 2018

Added schema spec for cost allocation tags #2742

Added schema spec for cost allocation tags #2742

Conversation

asarkar84 commented Mar 23, 2018 • edited Loading

PR information

Quality of Swagger

AutorestCI commented Mar 23, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Mar 23, 2018 • edited Loading

Automation for azure-libraries-for-java

AutorestCI commented Mar 23, 2018 • edited Loading

Automation for azure-sdk-for-node

AutorestCI commented Mar 23, 2018 • edited Loading

Automation for azure-sdk-for-go

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergey-shandar commented Mar 23, 2018

sergey-shandar commented Mar 23, 2018

amarzavery commented Mar 23, 2018

sergey-shandar commented Mar 26, 2018

asarkar84 commented Mar 26, 2018

Choose a reason for hiding this comment

asarkar84 Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asarkar84 commented Mar 27, 2018

amarzavery commented Mar 27, 2018

asarkar84 commented Mar 27, 2018

asarkar84 commented Apr 5, 2018

ravbhatnagar commented Apr 5, 2018

asarkar84 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

AutorestCI commented Mar 23, 2018 •

edited

Loading

asarkar84 Mar 27, 2018 •

edited

Loading