Add version 2018-06-01 with pricing configuration (second time) #4978

amireluk · 2019-01-03T11:58:44Z

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

I have reviewed the documentation for the workflow.
Validation tools were run on swagger spec(s) and have all been fixed in this PR.
The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
adding/removing APIs.
adding/removing properties.
adding/removing API-version.
adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Please follow the link to find more details on API review process.

openapi-portal-comment · 2019-01-03T11:58:45Z

If you're a MSFT employee, click this link
to view this PR's validation status on our new OpenAPI Hub spec management tool.

AutorestCI · 2019-01-03T11:59:13Z

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

azuresdkci · 2019-01-03T11:59:23Z

Can one of the admins verify this patch?

AutorestCI · 2019-01-03T11:59:25Z

Automation for azure-sdk-for-js

Nothing to generate for azure-sdk-for-js

AutorestCI · 2019-01-03T11:59:32Z

Automation for azure-sdk-for-python

Nothing to generate for azure-sdk-for-python

AutorestCI · 2019-01-03T11:59:37Z

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

amireluk · 2019-01-03T11:59:59Z

2nd iteration of this PR which was closed - #4072
With slight change to the PUT operation to reflect the API.

AutorestCI · 2019-01-03T12:00:55Z

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

AutorestCI · 2019-01-03T12:02:27Z

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

orparnes · 2019-01-03T12:07:30Z

@chlahav is added to the review. #Closed

amireluk · 2019-01-07T11:59:13Z

@kpajdzik Can you please review the PR ?

sergey-shandar · 2019-01-07T19:25:18Z

@praries880 see also this PR #4072

sergey-shandar · 2019-01-08T18:51:36Z

@amireluk have you addressed ARM issues (see #4072)?

@simongdavies let me know if you are ok with the changes.

amireluk · 2019-01-08T19:08:03Z

@amireluk have you addressed ARM issues (see #4072)?

@simongdavies let me know if you are ok with the changes.

ARM issues in #4072 was about long running operation header. In this iteration the operation is not a long running operation. It returns 200 OK.

ravbhatnagar

@amireluk - some comments

ravbhatnagar · 2019-01-09T23:47:04Z

specification/security/resource-manager/Microsoft.Security/stable/2018-06-01/security.json

+                    "description": "List of pricing configurations",
+                    "items": {
+                        "$ref": "#/definitions/Pricing"
+                    }


do you support pagination?

no, the number of returned items is limited.

ravbhatnagar · 2019-01-09T23:48:08Z

specification/security/resource-manager/Microsoft.Security/stable/2018-06-01/security.json

+            "type": "object",
+            "description": "Pricing properties for the relevant scope",
+            "properties": {
+                "pricingTier": {


Isnt this similar notion as a SKU? If yes, it should be modeled using the sku property which is a top level property. More details can be found in ARM Resource provider contract.

It's a new API version to an existing type, customers are using it and used to use it. I don't think we want to deprecate it and use the SKU propery instead as it will be very confusing to existing customers.
I'm also not sure we can use the SKU property even if we wanted to since this pricing type implies the tier of the subscritpion in Microsoft.Secruity, and not the sku of a specific resource.

ravbhatnagar · 2019-01-09T23:50:44Z

specification/security/resource-manager/Microsoft.Security/stable/2018-06-01/security.json

+            }
+        }
+    },
+    "paths": {


Did you had a meeting earlier with Simon about this new resource type? I wanted to get some more context on the usecase for the pricing resource type. what scenario is it enabling.

Please note it's an existing type, I'm adding a new API version. The previous version is already documented.

ravbhatnagar · 2019-01-10T21:01:05Z

@amireluk - Got it, it was hard to review using the diff tool and know what changed. For next time, please remember that when adding new api-version, the first commit should be the copy of the previous api-version. and then changes to the new api-version should come in subsequent commits. it will make it easier to review.
It looks like you are just moving /pricings resourceType to a non-preview api-version. And there are no changes to this existing resource type from preview. Is that correct.
Also, this means the customer will not be able to use the other resource types like securityContacts etc in this new api-version. Is that the intention?

amireluk · 2019-01-14T06:38:50Z

Also, this means the customer will not be able to use the other resource types like securityContacts etc in this new api-version. Is that the intention?

The non-preview pricing API is slightly different than the preview one, that's the reason for the version change.
I understand that this API version will only include this resource. The other resources have not changed.

KrisBash · 2019-01-15T20:42:28Z

I understand that this API version will only include this resource. The other resources have not changed.

@amireluk Can you please bring forward the unchanged APIs into this version? It creates a degraded user experience if different resources in the same namespace require different API versions.

amireluk · 2019-01-20T12:28:36Z

@KrisBash
The reason for a new api version for pricing resource is that it changes from the old one. The other did not change and we want to keep them in their existing preview api version.
I want to publish the changes in this PR so customers can have documentation for the pricing resource, even if it means the SDK will not be optimal. At this points the (already public) pricing api is not documented.

sergey-shandar · 2019-01-31T19:16:48Z

@KrisBash @amireluk any progress on this PR?

chlahav · 2019-02-07T11:50:06Z

@KrisBash we want to split our swagger files to types (similar to Compute) to mitigate the degraded user experience.
we will do that right after this merge will happen to avoid merge conflicts.
is there anything additional that we need to do to get the PR approval?

kpajdzik added the review label Jan 3, 2019

azuresdkci assigned praries880 Jan 3, 2019

azuresdkci requested a review from praries880 January 3, 2019 11:59

chlahav self-requested a review January 6, 2019 14:38

praries880 added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required APIStewardshipBoard-ReviewRequested This should be reviewed by the Azure API Stewardship team in partnership with the service team. labels Jan 7, 2019

kpajdzik assigned sergey-shandar Jan 8, 2019

sergey-shandar requested review from simonjaeger and simongdavies and removed request for simonjaeger January 8, 2019 18:51

sergey-shandar approved these changes Jan 9, 2019

View reviewed changes

ravbhatnagar reviewed Jan 9, 2019

View reviewed changes

ravbhatnagar added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Jan 9, 2019

ravbhatnagar mentioned this pull request Jan 10, 2019

Adding new api-version to Microsoft.Security Alerts resource #4902

Merged

5 tasks

second time - Add version 2018-06-01 with pricing configuration

2ffbe3c

chlahav force-pushed the master branch from c9bdece to 2ffbe3c Compare February 7, 2019 11:47

chlahav approved these changes Feb 7, 2019

View reviewed changes

sergey-shandar added the ARM-overdue ARM review has not occurred within the expected timeframe label Feb 7, 2019

sergey-shandar merged commit 277de73 into Azure:master Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add version 2018-06-01 with pricing configuration (second time) #4978

Add version 2018-06-01 with pricing configuration (second time) #4978

amireluk commented Jan 3, 2019 •

edited

Loading

openapi-portal-comment bot commented Jan 3, 2019

AutorestCI commented Jan 3, 2019 •

edited

Loading

azuresdkci commented Jan 3, 2019

AutorestCI commented Jan 3, 2019 •

edited

Loading

AutorestCI commented Jan 3, 2019 •

edited

Loading

AutorestCI commented Jan 3, 2019 •

edited

Loading

amireluk commented Jan 3, 2019

AutorestCI commented Jan 3, 2019 •

edited

Loading

AutorestCI commented Jan 3, 2019 •

edited

Loading

orparnes commented Jan 3, 2019

amireluk commented Jan 7, 2019

sergey-shandar commented Jan 7, 2019

sergey-shandar commented Jan 8, 2019 •

edited

Loading

amireluk commented Jan 8, 2019

ravbhatnagar left a comment

ravbhatnagar Jan 9, 2019

amireluk Jan 10, 2019

ravbhatnagar Jan 9, 2019

amireluk Jan 10, 2019

ravbhatnagar Jan 9, 2019

amireluk Jan 10, 2019

ravbhatnagar commented Jan 10, 2019

amireluk commented Jan 14, 2019

KrisBash commented Jan 15, 2019

amireluk commented Jan 20, 2019

sergey-shandar commented Jan 31, 2019

chlahav commented Feb 7, 2019

Add version 2018-06-01 with pricing configuration (second time) #4978

Add version 2018-06-01 with pricing configuration (second time) #4978

Conversation

amireluk commented Jan 3, 2019 • edited Loading

Latest improvements:

Contribution checklist:

ARM API Review Checklist

openapi-portal-comment bot commented Jan 3, 2019

AutorestCI commented Jan 3, 2019 • edited Loading

Automation for azure-sdk-for-ruby

azuresdkci commented Jan 3, 2019

AutorestCI commented Jan 3, 2019 • edited Loading

Automation for azure-sdk-for-js

AutorestCI commented Jan 3, 2019 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Jan 3, 2019 • edited Loading

Automation for azure-sdk-for-java

amireluk commented Jan 3, 2019

AutorestCI commented Jan 3, 2019 • edited Loading

Automation for azure-sdk-for-go

AutorestCI commented Jan 3, 2019 • edited Loading

Automation for azure-sdk-for-node

orparnes commented Jan 3, 2019

amireluk commented Jan 7, 2019

sergey-shandar commented Jan 7, 2019

sergey-shandar commented Jan 8, 2019 • edited Loading

amireluk commented Jan 8, 2019

ravbhatnagar left a comment

Choose a reason for hiding this comment

ravbhatnagar Jan 9, 2019

Choose a reason for hiding this comment

amireluk Jan 10, 2019

Choose a reason for hiding this comment

ravbhatnagar Jan 9, 2019

Choose a reason for hiding this comment

amireluk Jan 10, 2019

Choose a reason for hiding this comment

ravbhatnagar Jan 9, 2019

Choose a reason for hiding this comment

amireluk Jan 10, 2019

Choose a reason for hiding this comment

ravbhatnagar commented Jan 10, 2019

amireluk commented Jan 14, 2019

KrisBash commented Jan 15, 2019

amireluk commented Jan 20, 2019

sergey-shandar commented Jan 31, 2019

chlahav commented Feb 7, 2019

amireluk commented Jan 3, 2019 •

edited

Loading

AutorestCI commented Jan 3, 2019 •

edited

Loading

AutorestCI commented Jan 3, 2019 •

edited

Loading

AutorestCI commented Jan 3, 2019 •

edited

Loading

AutorestCI commented Jan 3, 2019 •

edited

Loading

AutorestCI commented Jan 3, 2019 •

edited

Loading

AutorestCI commented Jan 3, 2019 •

edited

Loading

sergey-shandar commented Jan 8, 2019 •

edited

Loading