Skip to content

Update subscription creation ARM API in Microsoft.Subscription RP#2535

Merged
lmazuel merged 22 commits intoAzure:masterfrom
amagams:master
Mar 20, 2018
Merged

Update subscription creation ARM API in Microsoft.Subscription RP#2535
lmazuel merged 22 commits intoAzure:masterfrom
amagams:master

Conversation

@amagams
Copy link
Contributor

@amagams amagams commented Feb 23, 2018

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

@AutorestCI
Copy link

AutorestCI commented Feb 23, 2018

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

@AutorestCI
Copy link

AutorestCI commented Feb 23, 2018

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

@wilcobmsft
Copy link
Member

This is awaiting ARM feedback - please add the "WaitForARMFeedback" label to this PR.

]
}
},
"/{scope}/providers/Microsoft.Subscription/createSubscription": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there are only a few of these, please create an instance for each parent resource type.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up offline on this. When we change the path to "/providers/Microsoft.Billing/billingAccounts/{billingAccountName}/providers/Microsoft.Subscription/createSubscription", autorest produces the following validation error:

ERROR (UniqueResourcePaths/R2059/RPCViolation): Multiple resource providers are not allowed in a single spec. More than one the resource paths were found: 'Microsoft.Subscription, Microsoft.Billing'.

"items": {
"$ref": "#/definitions/AdPrincipal"
},
"description": "The list of principals that should be granted Owner access on the subscription."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add to the description that the principals must be part of the same tenant.

Editing readme.md file and updating swagger path for Http Post
@@ -0,0 +1,35 @@
{
"parameters": {
"scope": "/providers/Microsoft.Billing/billingAccounts/a460e639-9c9b-4b58-9046-1120b22c7dc1/",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the examples to reflect your latest change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -0,0 +1,28 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename this file to "getSubscriptionOperations.json"? I think we'll also want to return (recently) completed operations for a better developer experience.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

"displayName": "Test Ea Azure Sub",
"owners": [
{
"tenantId": "73f7ab6e-cfa0-42be-b506-be6e77c2700c",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also change this to 05053bdf-7e6a-4f96-854f-1dae531c170b - we likely won't allow owners from different tenants to be assigned owner on the subscription.

Copy link
Member

@wilcobmsft wilcobmsft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure all checks are passing.

"operationId": "SubscriptionOperations_List",
"x-ms-examples": {
"getPendingSubscriptionOperations": {
"$ref": "./examples/getPendingSubscriptionOperations.json"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to getSubscriptionOperations (both file reference and the example name)

@alvadb
Copy link

alvadb commented Mar 7, 2018

I see that there hasn't been any activity on this PR for a week. Is this PR still valid?

@amagams
Copy link
Contributor Author

amagams commented Mar 7, 2018 via email

@AutorestCI
Copy link

AutorestCI commented Mar 9, 2018

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@@ -0,0 +1,35 @@
{
"parameters": {
"billingAccountName": "73f8ab6e-cfa0-42be-b886-be6e77c2980c",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enrollmentAccountName

{
"name": "enrollmentAccountName",
"in": "path",
"description": "The name of the billing account to which the subscription will be billed.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The name of the enrollment account ..."

@alvadb alvadb added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Mar 9, 2018
@jhendrixMSFT
Copy link
Member

@AutorestCI rebuild azure-sdk-for-go

1 similar comment
@jhendrixMSFT
Copy link
Member

@AutorestCI rebuild azure-sdk-for-go

…oft.Subscriptions swagger so that --package-all-subscriptions tag can produce a single package
@@ -0,0 +1,312 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename file to "subscriptions.json" to be consistent with the (existing) stable version.

"swagger": "2.0",
"info": {
"version": "2018-03-01-preview",
"title": "SubscriptionManagementClient",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to "SubscriptionClient" for consistency with the stable version.

"info": {
"version": "2018-03-01-preview",
"title": "SubscriptionManagementClient",
"description": "Subscription management client provides an interface to create and manage Azure subscriptions programmatically."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge with the description of the stable version. Drop the term "management client".

@lmazuel lmazuel assigned lmazuel and alvadb and unassigned alvadb Mar 19, 2018
@lmazuel lmazuel merged commit 2b23b99 into Azure:master Mar 20, 2018
@jhendrixMSFT
Copy link
Member

@AutorestCI regenerate azure-sdk-for-go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants