Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Blueprints API update for excludedPrincipals and whoIsBlueprint #5470

Merged
merged 16 commits into from
Mar 28, 2019
Merged

Blueprints API update for excludedPrincipals and whoIsBlueprint #5470

merged 16 commits into from
Mar 28, 2019

Conversation

filizt
Copy link
Contributor

@filizt filizt commented Mar 25, 2019 •
edited
Loading

This PR includes two updates to Blueprints API specification:

  1. Add a property to AssignmentLockSettings called excludedPrincipals.
  2. Add a support for Post operation called WhoIsBlueprints.

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.

@AutorestCI
Copy link

AutorestCI commented Mar 25, 2019 •
edited
Loading

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

@AutorestCI
Copy link

AutorestCI commented Mar 25, 2019 •
edited
Loading

Automation for azure-sdk-for-js

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-js#1849

@AutorestCI
Copy link

AutorestCI commented Mar 25, 2019 •
edited
Loading

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI AutorestCI mentioned this pull request Mar 25, 2019
Merged
@AutorestCI
Copy link

AutorestCI commented Mar 25, 2019 •
edited
Loading

Automation for azure-sdk-for-python

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

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Mar 25, 2019 •
edited
Loading

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@filizt filizt changed the title Blueprints API update for excludePrincipals and whoIsBlueprint Blueprints API update for excludedPrincipals and whoIsBlueprint Mar 25, 2019
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 25, 2019
@adxsdknet
.NET SDK Resource Provider:'Blueprint'
e62a259 
REST Spec PR 'Azure/azure-rest-api-specs#5470'
REST Spec PR Author 'filizt'
REST Spec PR Last commit
@adxsdknet
Copy link

adxsdknet commented Mar 25, 2019 •
edited
Loading

Automation for azure-sdk-for-net

A PR has been created for you:
Azure/azure-sdk-for-net#5580
.NET SDK Commits:
adxsdknet/azure-sdk-for-net@e62a259
adxsdknet/azure-sdk-for-net@083bb8d
adxsdknet/azure-sdk-for-net@81fb68d
adxsdknet/azure-sdk-for-net@00d93e7
adxsdknet/azure-sdk-for-net@14d7510
adxsdknet/azure-sdk-for-net@a2b77f5
adxsdknet/azure-sdk-for-net@cfd1d2a
adxsdknet/azure-sdk-for-net@f9441de

majastrz
majastrz reviewed Mar 25, 2019
View reviewed changes
...int/resource-manager/Microsoft.Blueprint/preview/2018-11-01-preview/blueprintAssignment.json Outdated
@@ -525,6 +550,15 @@
"description": "Last modified time of this blueprint definition."
}
}
},
"WhoIsBlueprintContract":{
"description": "AAD object Id of the Azure Blueprints service principal in the tenant.",
Copy link
Member

@majastrz majastrz Mar 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

AAD object Id of the Azure Blueprints service principal in the tenant. [](start = 22, length = 70)

This description belongs on the property probably. #Closed

@majastrz majastrz added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review labels Mar 25, 2019
majastrz
majastrz requested changes Mar 25, 2019
View reviewed changes
Copy link
Member

@majastrz majastrz left a comment

Choose a reason for hiding this comment

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

Overall looks good. Can you make the string changes I suggested, though?

jhendrixMSFT
jhendrixMSFT requested changes Mar 25, 2019
View reviewed changes
Copy link
Member

@jhendrixMSFT jhendrixMSFT left a comment

Choose a reason for hiding this comment

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

Please fix operation ID

...int/resource-manager/Microsoft.Blueprint/preview/2018-11-01-preview/blueprintAssignment.json Outdated Show resolved Hide resolved
...int/resource-manager/Microsoft.Blueprint/preview/2018-11-01-preview/blueprintAssignment.json Outdated Show resolved Hide resolved
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 26, 2019
@adxsdknet
.NET SDK Resource Provider:'Blueprint'
083bb8d 
REST Spec PR 'Azure/azure-rest-api-specs#5470'
REST Spec PR Author 'filizt'
REST Spec PR Last commit
@majastrz majastrz added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Mar 26, 2019
@filizt
Copy link
Contributor Author

filizt commented Mar 27, 2019

@filizt, Regarding Node SDK - could you change - repo: azure-sdk-for-node to - repo: azure-sdk-for-js in main readme.md? Node SDK will be deprecated in June/July and we're not releasing any new packages for it. The -js SDKs are written in TypeScript and work both in Node.js and browser.
Also, we're missing language specific readmes. @jhendrixMSFT, did you add the go one?

@kpajdzik I updated the read me for the azure-sdk-for-js package name.

We already had the go readme in the repo, should we add readmes for pyton and js as well? Are they required?

@kpajdzik
Copy link
Contributor

kpajdzik commented Mar 27, 2019 •
edited
Loading

@filizt, Regarding Node SDK - could you change - repo: azure-sdk-for-node to - repo: azure-sdk-for-js in main readme.md? Node SDK will be deprecated in June/July and we're not releasing any new packages for it. The -js SDKs are written in TypeScript and work both in Node.js and browser.
Also, we're missing language specific readmes. @jhendrixMSFT, did you add the go one?

@kpajdzik I updated the read me for the azure-sdk-for-js package name.

We already had the go readme in the repo, should we add readmes for pyton and js as well? Are they required?

Yes, they are required if you want to generate package in that particular language. As I cannot push to your fork, could you copy readme.typescript.md from other services like https://github.com/Azure/azure-rest-api-specs/blob/master/specification/eventgrid/resource-manager/readme.typescript.md? Just change the name and the output folder. I think it should be similar for Python.

@AutorestCI AutorestCI mentioned this pull request Mar 27, 2019
Closed
kpajdzik and others added 2 commits March 27, 2019 14:23
@kpajdzik @filizt
Update specification/blueprint/resource-manager/readme.typescript.md
fce7e71 
Co-Authored-By: filizt <filizkurban@gmail.com>
@kpajdzik @filizt
Update specification/blueprint/resource-manager/readme.typescript.md
f7379ca 
Co-Authored-By: filizt <filizkurban@gmail.com>
@kpajdzik
Copy link
Contributor

LGTM

adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 27, 2019
@adxsdknet
.NET SDK Resource Provider:'Blueprint'
14d7510 
REST Spec PR 'Azure/azure-rest-api-specs#5470'
REST Spec PR Author 'filizt'
REST Spec PR Last commit
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 27, 2019
@adxsdknet
.NET SDK Resource Provider:'Blueprint'
a2b77f5 
REST Spec PR 'Azure/azure-rest-api-specs#5470'
REST Spec PR Author 'filizt'
REST Spec PR Last commit
@AutorestCI AutorestCI mentioned this pull request Mar 28, 2019
Closed
@filizt
Copy link
Contributor Author

filizt commented Mar 28, 2019

Please fix operation ID

Fixed earlier.

@filizt filizt closed this Mar 28, 2019
@filizt filizt reopened this Mar 28, 2019
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 28, 2019
@adxsdknet
.NET SDK Resource Provider:'Blueprint'
cfd1d2a 
REST Spec PR 'Azure/azure-rest-api-specs#5470'
REST Spec PR Author 'filizt'
REST Spec PR Last commit
@AutorestCI AutorestCI mentioned this pull request Mar 28, 2019
Closed
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 28, 2019
@adxsdknet
.NET SDK Resource Provider:'Blueprint'
f9441de 
REST Spec PR 'Azure/azure-rest-api-specs#5470'
REST Spec PR Author 'filizt'
REST Spec PR Last commit
@jhendrixMSFT jhendrixMSFT merged commit a8b96bf into Azure:master Mar 28, 2019
@AutorestCI AutorestCI mentioned this pull request Mar 28, 2019
Merged
@jhendrixMSFT
Copy link
Member

@AutorestCI regenerate azure-sdk-for-go

@AutorestCI AutorestCI mentioned this pull request Mar 29, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmazuel lmazuel lmazuel left review comments

@kpajdzik kpajdzik kpajdzik left review comments

@jhendrixMSFT jhendrixMSFT jhendrixMSFT approved these changes

@majastrz majastrz majastrz approved these changes

Assignees

@jhendrixMSFT jhendrixMSFT

Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@filizt @AutorestCI @azuresdkci @adxsdknet @jhendrixMSFT @kpajdzik @lmazuel @majastrz