Above-Subscription Level Marketplace and Usage Detail, and Enrollment Level Balances #2857

parisa-naeimi · 2018-04-12T05:50:27Z

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.

… Account) Marketplace, Usage Details, and Enrollment Level Balances.

msftclas · 2018-04-12T05:50:38Z

All CLA requirements met.

AutorestCI · 2018-04-12T05:51:04Z

Automation for azure-libraries-for-java

Nothing to generate for azure-libraries-for-java

AutorestCI · 2018-04-12T05:51:21Z

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

AutorestCI · 2018-04-12T05:51:25Z

Automation for azure-sdk-for-node

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-node#2700

AutorestCI · 2018-04-12T05:53:59Z

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

marstr · 2018-04-12T16:54:55Z

Howdy @parisa-naeimi,

Before we get started, it looks like there are numerous issues being flagged by our CI with this PR. Including some of these files not being valid JSON. Those issues will need to be fixed before the ARM team and I can get started with out reviews.

You can see detailed information about the errors in the CI Logs, here:
https://travis-ci.org/Azure/azure-rest-api-specs/builds/365454151?utm_source=github_status&utm_medium=notification

…laced priceHidden boolean type with enum for more clarity

parisa-naeimi · 2018-04-13T19:00:45Z

@marstr Could you please take a look at my PR?

parisa-naeimi · 2018-04-16T18:05:47Z

@ms-premp Just for awareness.

parisa-naeimi · 2018-04-16T18:28:48Z

@marstr Hello Martin, I was wondering if you could review this PR? Thank you very much.

marstr · 2018-04-16T23:47:53Z

Howdy @parisa-naeimi, sorry for the delay. I'll take another look at this PR and have comments in by EOD tomorrow.

parisa-naeimi · 2018-04-16T23:49:50Z

@marstr, Sure, thank you very much.

marstr

CI is still reporting a new model error: https://travis-ci.org/Azure/azure-rest-api-specs/jobs/365866211#L756 Specifically it looks like there is an unknown property "value" in the examples. I'm not sure if it's BalancesByBillingAccount.json#L31 or BalancesByBillingAccount.json#L41, but one of the defintions will need to be tweaked to accommodate that property.

marstr · 2018-04-17T20:43:22Z

Thanks for fixing up the syntax errors, I've white-listed this PR for ARM Feedback.

azuresdkciprbot · 2018-04-17T23:48:38Z

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:

💡 Please review potentially introduced Error(s)/Warning(s): Analysis Report 💡

File: specification/consumption/resource-manager/readme.md
Before the PR: Warning(s): 11 Error(s): 0
After the PR: Warning(s): 11 Error(s): 3

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

…er-location to some parameters missing that part.

azuresdkciprbot · 2018-04-18T00:18:07Z

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

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

parisa-naeimi · 2018-04-18T00:25:51Z

@marstr Thank you very much for taking the time and looking at this PR. I already addressed your concern for BalancesByBillingAccount.Json, could you please take a look and let me know if I need to do more revision?

ravbhatnagar · 2018-04-19T07:16:11Z

@parisa-naeimi - please schedule a review with ARM and Azure API review board to go over these APIs. There are some new concepts like department etc which need little more clarity from a scenario pov.
@devigned @johanste @darrelmiller FYI

marstr · 2018-04-19T16:55:57Z

I'm going to wait to review further until the Azure API Review board has had a chance to look over these changes.

ravbhatnagar · 2018-04-25T22:39:44Z

Signing off! We did meet to go over these apis

parisa-naeimi · 2018-04-25T22:42:08Z

@marstr
Hello Martin,
Arm team already signed off this PR, Could you please take a look at this PR?

Thank You,
Parisa

# Conflicts: # specification/consumption/resource-manager/Microsoft.Consumption/stable/2018-03-31/consumption.json # specification/consumption/resource-manager/Microsoft.Consumption/stable/2018-03-31/examples/CostTags.json # specification/consumption/resource-manager/Microsoft.Consumption/stable/2018-03-31/examples/CreateOrUpdateCostTags.json

azuresdkciprbot · 2018-04-26T02:45:06Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/consumption/resource-manager/readme.md

⚠️

0 new Warnings.(4 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-26T02:45:06Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/consumption/resource-manager/readme.md

⚠️

0 new Warnings.(7 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

marstr · 2018-04-26T21:28:05Z

I see that there are a lot of operations, that look to actually be going through the Microsoft.Billing RP, but targeting a Resource that you control in Consumption. See here for an example of what I'm talking about:
https://github.com/Azure/azure-rest-api-specs/pull/2857/files#diff-dca9b11ab859f276378213344e091b34R304

Could you explain quickly why these operations belong in your Swagger? is there a tight relationship between these two RPs? I'll have a chat with other Swagger reviewers to make sure this isn't just a knowledge gap of mine.

ms-premp · 2018-04-26T21:44:53Z

@marstr .. unfortunately the diff tool suggest there are a lot of changes but its actually not.
I can schedule a meeting with you to go over it if that's needed.
In a nutshell, there are three APIs
UsageDetails
MarketPlaces
Balances
These already exist today and users call them under subscription scope.. like
/subscription/{subscriptionGuid}/providers/Microsoft.Consumption/usageDetails
also user can specify a billing period scope like
/subscription/{subscriptionGuid}/providers/Microsoft.Billing/billingperiods/{billingPeriodId}/providers/Microsoft.Consumption/usageDetails

These already exist today.
The new change here is that we want to provide same APIs but not at scope of subscription.. at a higher scope like billingAccounts, departments or enrollmentAccounts.
The response does not change at all. Its still the same.
The change is the request scope.
A sample request will look like
/providers/Microsoft.Billing/billingAccounts/{EnrollmentNumber}/provider/Microsoft.Consumption/usageDetails.
So the scope here is "providers/Microsoft.Billing/billingAccounts/{EnrollmentNumber}" and not "/subscriptions/{subscriptionGuid}" as before.
The proxy resources such as billingAccounts, departments, enrollmentAccounts are defined and owned by Microsoft.Billing RP and so any scope or extension to those resources will have to include those in Request.
Our Consumption RP is registered to behave as an extension to any other RP resources, so that usage data can be scoped to a specific RP resource.
This Extension routing is supported in ARM since very beginning and is used by multiple teams
see extension at
http://sharepoint/sites/AzureUX/Sparta/SpartaWiki/Resource%20Provider%20Registration%20Routing%20Types.aspx
Hope, this answers your queries.

marstr · 2018-04-26T21:58:18Z

Thanks for that explanation, @ms-premp, that is much appreciated.

marstr

Whoops, sorry for the false approval. Looks like there are couple of examples that don't match the operation's declaration.

Mind fixing those up quick?
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/371363427#L768

Other than that, this looks good to me.

azuresdkciprbot · 2018-04-27T00:05:30Z

AutoRest linter results for SDK Related Validation Errors/Warnings

These errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns.

File: specification/consumption/resource-manager/readme.md

⚠️

0 new Warnings.(4 total)

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-27T00:05:30Z

AutoRest linter results for ARM Related Validation Errors/Warnings

These errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns.

File: specification/consumption/resource-manager/readme.md

⚠️

1 new Warnings.(8 total)

Code	Id	Source	Message
EnumInsteadOfBoolean	R3018	Link	Booleans are not descriptive and make them hard to use. Consider using string enums with allowed set of values defined. Property: priceHidden

❌0 new Errors.(0 total)

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

parisa-naeimi · 2018-04-27T00:14:56Z

@marstr

Hello Martin,

Thank you very much for taking the time and looking at this PR. I fixed those minor issues in examples and checked in the changes. Please let me know if there is any concerns. I appreciate if you take a look at PR.

Thank you again,
Parisa

marstr · 2018-04-30T23:11:17Z

@parisa-naeimi, are these changes deployed to at least one region and ready for me to merge them?

parisa-naeimi · 2018-04-30T23:17:09Z

@marstr Yes, those changes are in Prod. we deployed to Prod today. We should be good. Thank you.

parisa-naeimi · 2018-05-01T00:37:12Z

@marstr Thank you Martin

parisa-naeimi added 2 commits April 11, 2018 22:15

This PR includes above-subscription level (Enrollment, Department, or…

0d10889

… Account) Marketplace, Usage Details, and Enrollment Level Balances.

Merge branch 'master' of https://github.com/Azure/azure-rest-api-specs

e8a464b

azuresdkci assigned marstr Apr 12, 2018

azuresdkci requested a review from marstr April 12, 2018 05:51

Updated consumption.json to fix build failure issues

247831c

Added "Get" to Operation Name 'BalancesByBillingAccount' and also rep…

a198973

…laced priceHidden boolean type with enum for more clarity

marstr requested changes Apr 17, 2018

View reviewed changes

marstr added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 17, 2018

Added Enrollment-Level balances by BillingPeriod

c70d23f

AutorestCI mentioned this pull request Apr 17, 2018

[AutoPR consumption/resource-manager] Above-Subscription Level Marketplace and Usage Detail, and Enrollment Level Balances Azure/azure-sdk-for-go#1631

Closed

Updated BalancesByBillingAccount example files and added x-ms-paramet…

f00a550

…er-location to some parameters missing that part.

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 25, 2018

parisa-naeimi added 2 commits April 25, 2018 17:08

Replaced Microsoft.CostManagment with Microsoft.Billing

876f31a

marstr approved these changes Apr 26, 2018

View reviewed changes

marstr requested changes Apr 26, 2018

View reviewed changes

Fixed example errors

d53bf42

marstr approved these changes Apr 30, 2018

View reviewed changes

marstr merged commit a0f2b94 into Azure:master Apr 30, 2018

AutorestCI mentioned this pull request Apr 30, 2018

[AutoPR consumption/resource-manager] Above-Subscription Level Marketplace and Usage Detail, and Enrollment Level Balances Azure/azure-sdk-for-go#1737

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Above-Subscription Level Marketplace and Usage Detail, and Enrollment Level Balances #2857

Above-Subscription Level Marketplace and Usage Detail, and Enrollment Level Balances #2857

parisa-naeimi commented Apr 12, 2018 •

edited by azuresdkci

Loading

msftclas commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

marstr commented Apr 12, 2018

parisa-naeimi commented Apr 13, 2018

parisa-naeimi commented Apr 16, 2018

parisa-naeimi commented Apr 16, 2018

marstr commented Apr 16, 2018

parisa-naeimi commented Apr 16, 2018

marstr left a comment

marstr commented Apr 17, 2018

azuresdkciprbot commented Apr 17, 2018

azuresdkciprbot commented Apr 18, 2018

parisa-naeimi commented Apr 18, 2018

ravbhatnagar commented Apr 19, 2018

marstr commented Apr 19, 2018

ravbhatnagar commented Apr 25, 2018

parisa-naeimi commented Apr 25, 2018

azuresdkciprbot commented Apr 26, 2018

azuresdkciprbot commented Apr 26, 2018

marstr commented Apr 26, 2018

ms-premp commented Apr 26, 2018

marstr commented Apr 26, 2018

marstr left a comment

azuresdkciprbot commented Apr 27, 2018

azuresdkciprbot commented Apr 27, 2018

parisa-naeimi commented Apr 27, 2018

marstr commented Apr 30, 2018

parisa-naeimi commented Apr 30, 2018

parisa-naeimi commented May 1, 2018

Above-Subscription Level Marketplace and Usage Detail, and Enrollment Level Balances #2857

Above-Subscription Level Marketplace and Usage Detail, and Enrollment Level Balances #2857

Conversation

parisa-naeimi commented Apr 12, 2018 • edited by azuresdkci Loading

PR information

Quality of Swagger

msftclas commented Apr 12, 2018 • edited Loading

AutorestCI commented Apr 12, 2018 • edited Loading

Automation for azure-libraries-for-java

AutorestCI commented Apr 12, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Apr 12, 2018 • edited Loading

Automation for azure-sdk-for-node

AutorestCI commented Apr 12, 2018 • edited Loading

Automation for azure-sdk-for-go

marstr commented Apr 12, 2018

parisa-naeimi commented Apr 13, 2018

parisa-naeimi commented Apr 16, 2018

parisa-naeimi commented Apr 16, 2018

marstr commented Apr 16, 2018

parisa-naeimi commented Apr 16, 2018

marstr left a comment

Choose a reason for hiding this comment

marstr commented Apr 17, 2018

azuresdkciprbot commented Apr 17, 2018

azuresdkciprbot commented Apr 18, 2018

parisa-naeimi commented Apr 18, 2018

ravbhatnagar commented Apr 19, 2018

marstr commented Apr 19, 2018

ravbhatnagar commented Apr 25, 2018

parisa-naeimi commented Apr 25, 2018

azuresdkciprbot commented Apr 26, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

azuresdkciprbot commented Apr 26, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

marstr commented Apr 26, 2018

ms-premp commented Apr 26, 2018

marstr commented Apr 26, 2018

marstr left a comment

Choose a reason for hiding this comment

azuresdkciprbot commented Apr 27, 2018

AutoRest linter results for SDK Related Validation Errors/Warnings

azuresdkciprbot commented Apr 27, 2018

AutoRest linter results for ARM Related Validation Errors/Warnings

parisa-naeimi commented Apr 27, 2018

marstr commented Apr 30, 2018

parisa-naeimi commented Apr 30, 2018

parisa-naeimi commented May 1, 2018

parisa-naeimi commented Apr 12, 2018 •

edited by azuresdkci

Loading

msftclas commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading