[Hub Generated] Review request for Microsoft.CostManagement to add version preview/2020-03-01-preview #10407

zachraMSFT · 2020-08-11T00:16:01Z

This is a PR generated at OpenAPI Hub. You can view your work branch via this link.

Contribution checklist:

I commit to follow the Breaking Change Policy of “no breaking changes
I have reviewed the documentation for the workflow.
Validation tools were run on swagger spec(s) and errors have all been fixed in this PR. How to fix?

If any further question about AME onboarding or validation tools, please view the FAQ.

ARM API Review Checklist

Ensure to check this box if one of the following scenarios meet updates in the PR, so that label “WaitForARMFeedback” will be added automatically to involve ARM API Review. Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs, all “removals” and “adding a new property” no more require ARM API review.
- Adding new API(s)
- Adding a new API version
- Adding a new service
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.

Breaking Change Review Checklist

If there are following updates in the PR, ensure to request an approval from API Review Board as defined in the Breaking Change Policy.

Removing API(s) in stable version
Removing properties in stable version
Removing API version(s) in stable version
Updating API in stable version with Breaking Change Validation errors
Updating API(s) in preview over 1 year

Please follow the link to find more details on PR review process.

…2019-04-01-preview to version 2020-03-01-preview

azure-pipelines · 2020-08-14T07:21:54Z

Azure Pipelines successfully started running 1 pipeline(s).

zachraMSFT · 2020-08-14T07:24:01Z

@chiragg4u Is this the proper configuration to avoid any deprecations and just add a new type with a new version from api perspective? Including the costmanagement.json in preview folder generated avocado errors so it seemed like this was the correct option, but want to make sure.

azure-pipelines · 2020-08-14T07:26:41Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2020-08-14T07:47:47Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2020-08-14T08:00:01Z

Azure Pipelines successfully started running 1 pipeline(s).

ravbhatnagar

@zachraMSFT few comments

ravbhatnagar · 2020-08-18T16:21:30Z

...ent/resource-manager/Microsoft.CostManagement/preview/2020-03-01-preview/costallocation.json

+        }
+      }
+    },
+    "/providers/Microsoft.Billing/billingAccounts/{billingAccountId}/providers/Microsoft.CostManagement/costAllocationRules/checkNameAvailability": {


Move the action checkNameAvailability directly under the provider, and then the resource type can be taken in the request body, this will let you use the same method for other resource types where you want to provide this functionality of check name. And it would be compliant with ARM Resource provuder contract -
https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/proxy-api-reference.md#check-name-availability-requests

Is this change required? We already reviewed having the field this way previously (https://github.com/Azure/azure-rest-api-specs-pr/pull/1152) and now have a number of dependencies based on this. Is this a recent change? I thought we configured per recommendations when we implemented it and did not specify resource type.

...ent/resource-manager/Microsoft.CostManagement/preview/2020-03-01-preview/costallocation.json

ravbhatnagar · 2020-08-18T16:25:51Z

...ent/resource-manager/Microsoft.CostManagement/preview/2020-03-01-preview/costallocation.json

+              "$ref": "#/definitions/RuleStatus",
+              "description": "Status of the rule"
+            },
+            "createdDate": {


there is a new top level property called systemData. and such fields are part of the schema there. This is dont for consistency sake so that any azure resource will have this data and in the same property. https://github.com/Azure/azure-rest-api-specs/blob/master/specification/common-types/resource-management/v2/types.json#L355
Other details here - https://armwiki.azurewebsites.net/api_contracts/ResourceSystemData.html?q=systemdata

Is this change required? We already reviewed having the field this way previously (https://github.com/Azure/azure-rest-api-specs-pr/pull/1152) and now have a number of dependencies based on this. Since we determined our setup for this before the change, it introduces lots of randomization on us to switch post development

ravbhatnagar · 2020-08-18T16:26:00Z

...ent/resource-manager/Microsoft.CostManagement/preview/2020-03-01-preview/costallocation.json

+              "format": "date-time",
+              "description": "Time at which the rule was created. Rules that change cost for the same resource are applied in order of creation."
+            },
+            "updatedDate": {


same as above

azure-pipelines · 2020-08-19T03:07:15Z

Azure Pipelines successfully started running 1 pipeline(s).

ravbhatnagar · 2020-08-19T17:35:29Z

Signing off as per below comments from Sachin
[9:55 AM] Sachin Srivastava
Is it possible to treat the comments in a separate PR at a later time. We had an earlier review and was approved at that time. Hence we made the API and UX with the existing contract.. and any change at this time would have a lot of ripple effect.
[9:56 AM] Sachin Srivastava
We are in public-preview state.. and have to set up another iteration before we go GA.

azure-pipelines · 2020-08-19T17:54:40Z

Azure Pipelines successfully started running 1 pipeline(s).

njuCZ · 2020-08-20T02:23:40Z

@zachraMSFT Could you please fix the CI error: LintDiff and Semantic?

azure-pipelines · 2020-08-20T09:29:30Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2020-08-20T09:55:11Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2020-08-20T18:40:44Z

Azure Pipelines successfully started running 1 pipeline(s).

zachraMSFT · 2020-08-20T18:47:57Z

@njuCZ I fixed the errors when I run validation independently, but looks like checks here are stuck

zachraMSFT added 30 commits June 10, 2020 21:25

Adds base for updating Microsoft.CostManagement from version preview/…

661ebb8

…2019-04-01-preview to version 2020-03-01-preview

Updates readme

727e1f4

Updates API version in new specs and examples

6c98a4f

Sync to private repo change set

ba3043d

Fix warnings

7d59c8d

Fix example for string over boolean

eba5f4b

List operation id fix

0dca2aa

Implement changes from review

cd1b1af

Fix response types

95f71bc

Only one tag

193c633

Fix failures

da876f5

Fix operation Ids

6740f12

Just CostAllocation

f4a466d

Percentage as number

5ac1d56

Revert to working version

fb2744e

First wave of review changes

88b6801

ruleName in examples

0f7f59c

Percentage as number, example for tag

c89db65

ruleName in tag example

8b3b0df

Add nesting for list

56de5cf

More changes

6c3be4b

Fix example name

bf0d063

Update tags/summary setup

d139ee6

Update based on review

a19bb98

Complete changes for API review

122c520

remove tags

e6070d8

fix response model, restore tag

b1eed58

Add 204 to example

788a09a

Remove actual next link until implemented

6ac27f6

Improve naming

2fc8414

Remove items that aren't changing versions

16b4675

Use 2020-06-01 to avoid lint errors

f60d21b

Fix more object mismatches

7d05743

Match parameter descriptions

032ba6b

ravbhatnagar reviewed Aug 18, 2020

View reviewed changes

ravbhatnagar added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Aug 18, 2020

Remove request/response differences

5141fda

Fix semantics

73d093a

Fix readonly setup

2bf0282

Remove format from readonly field

12494b4

Fix capitalization

6be6f49

openapi-sdkautomation bot mentioned this pull request Aug 20, 2020

[AutoPR cost-management] [Hub Generated] Review request for Microsoft.CostManagement to add version preview/2020-03-01-preview AzureSDKAutomation/azure-resource-manager-schemas#120

Closed

njuCZ approved these changes Aug 21, 2020

View reviewed changes

njuCZ merged commit 7ac0082 into Azure:master Aug 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Hub Generated] Review request for Microsoft.CostManagement to add version preview/2020-03-01-preview #10407

[Hub Generated] Review request for Microsoft.CostManagement to add version preview/2020-03-01-preview #10407

zachraMSFT commented Aug 11, 2020 •

edited

Loading

azure-pipelines bot commented Aug 14, 2020

zachraMSFT commented Aug 14, 2020

azure-pipelines bot commented Aug 14, 2020

azure-pipelines bot commented Aug 14, 2020

azure-pipelines bot commented Aug 14, 2020

ravbhatnagar left a comment

ravbhatnagar Aug 18, 2020

zachraMSFT Aug 18, 2020

ravbhatnagar Aug 18, 2020

zachraMSFT Aug 18, 2020

ravbhatnagar Aug 18, 2020

azure-pipelines bot commented Aug 19, 2020

ravbhatnagar commented Aug 19, 2020

azure-pipelines bot commented Aug 19, 2020

njuCZ commented Aug 20, 2020

azure-pipelines bot commented Aug 20, 2020

azure-pipelines bot commented Aug 20, 2020

azure-pipelines bot commented Aug 20, 2020

zachraMSFT commented Aug 20, 2020

[Hub Generated] Review request for Microsoft.CostManagement to add version preview/2020-03-01-preview #10407

[Hub Generated] Review request for Microsoft.CostManagement to add version preview/2020-03-01-preview #10407

Conversation

zachraMSFT commented Aug 11, 2020 • edited Loading

Contribution checklist:

ARM API Review Checklist

Breaking Change Review Checklist

azure-pipelines bot commented Aug 14, 2020

zachraMSFT commented Aug 14, 2020

azure-pipelines bot commented Aug 14, 2020

azure-pipelines bot commented Aug 14, 2020

azure-pipelines bot commented Aug 14, 2020

ravbhatnagar left a comment

Choose a reason for hiding this comment

ravbhatnagar Aug 18, 2020

Choose a reason for hiding this comment

zachraMSFT Aug 18, 2020

Choose a reason for hiding this comment

ravbhatnagar Aug 18, 2020

Choose a reason for hiding this comment

zachraMSFT Aug 18, 2020

Choose a reason for hiding this comment

ravbhatnagar Aug 18, 2020

Choose a reason for hiding this comment

azure-pipelines bot commented Aug 19, 2020

ravbhatnagar commented Aug 19, 2020

azure-pipelines bot commented Aug 19, 2020

njuCZ commented Aug 20, 2020

azure-pipelines bot commented Aug 20, 2020

azure-pipelines bot commented Aug 20, 2020

azure-pipelines bot commented Aug 20, 2020

zachraMSFT commented Aug 20, 2020

zachraMSFT commented Aug 11, 2020 •

edited

Loading