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.

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

[ACR] Add "19-05-01" GA specification for Microsoft.ContainerRegistry #5854

Merged
merged 8 commits into from
May 24, 2019

Conversation

ankurkhemani
Copy link
Contributor

@ankurkhemani ankurkhemani commented May 3, 2019

Latest improvements:

  • Remove support for PUT/PATCH of registry resource with Classic SKU.
  • Refactor Policy APIs: Policy is now part of RegistryProperties and hence managed by CRUD on registry resource.
  • PUT on ARM resources now supports replace semantics (no swagger change). // cc @ravbhatnagar
    (no new APIs added, two APIs deprecated)

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.

@ankurkhemani ankurkhemani changed the title add v19_05_GA swagger [ACR] Add v19_05_GA swagger May 3, 2019
@AutorestCI
Copy link

AutorestCI commented May 3, 2019

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

@AutorestCI
Copy link

AutorestCI commented May 3, 2019

Automation for azure-sdk-for-python

Encountered a Subprocess error: (azure-sdk-for-python)

Command: python ./scripts/multiapi_init_gen.py azure-mgmt-containerregistry
Finished with return code 1
and output:

Traceback (most recent call last):
  File "./scripts/multiapi_init_gen.py", line 22, in <module>
    import azure.common
ModuleNotFoundError: No module named 'azure'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./scripts/multiapi_init_gen.py", line 26, in <module>
    import azure.common
ModuleNotFoundError: No module named 'azure'

@AutorestCI
Copy link

AutorestCI commented May 3, 2019

Automation for azure-sdk-for-ruby

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

@azuresdkci azuresdkci requested a review from sergey-shandar May 3, 2019 02:35
@ankurkhemani ankurkhemani changed the title [ACR] Add v19_05_GA swagger [ACR] Add "19-05-01" GA specification for Microsoft.ContainerRegistry May 3, 2019
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented May 3, 2019

Automation for azure-sdk-for-go

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

@AutorestCI
Copy link

AutorestCI commented May 3, 2019

Automation for azure-sdk-for-java

Encountered a Subprocess error: (azure-sdk-for-java)

Command: ['/usr/local/bin/autorest', '/tmp/tmpizm0bk8b/rest/specification/containerregistry/resource-manager/readme.md', '--perform-load=false', '--swagger-to-sdk', '--output-artifact=configuration.json', '--input-file=foo', '--output-folder=/tmp/tmpcui6pzj4']
Finished with return code 7
and output:

AutoRest code generation utility [version: 2.0.4283; node: v8.12.0]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
Failure:
Error: Unable to start AutoRest Core from /root/.autorest/@microsoft.azure_autorest-core@2.0.4373/node_modules/@microsoft.azure/autorest-core
Error: Unable to start AutoRest Core from /root/.autorest/@microsoft.azure_autorest-core@2.0.4373/node_modules/@microsoft.azure/autorest-core
    at main (/opt/node_modules/autorest/dist/app.js:232:19)
    at <anonymous>

/root/.autorest/@microsoft.azure_autorest-core@2.0.4373/node_modules/@microsoft.azure/autorest-core/dist/app.js:33
    autorest_core_1.Shutdown();
    ^
ReferenceError: autorest_core_1 is not defined
    at process.on (/root/.autorest/@microsoft.azure_autorest-core@2.0.4373/node_modules/@microsoft.azure/autorest-core/dist/app.js:33:5)
    at emitOne (events.js:121:20)
    at process.emit (events.js:211:7)
    at process.emit (/node_modules/source-map-support/source-map-support.js:439:21)
fs.js:612
  return binding.close(fd);
                 ^

Error: EBADF: bad file descriptor, close
    at Object.fs.closeSync (fs.js:612:18)
    at StaticVolumeFile.shutdown (/opt/node_modules/autorest/dist/static-loader.js:352:10)
    at StaticFilesystem.shutdown (/opt/node_modules/autorest/dist/static-loader.js:406:17)
    at process.exit.n [as exit] (/opt/node_modules/autorest/dist/static-loader.js:169:11)
    at printErrorAndExit (/node_modules/source-map-support/source-map-support.js:423:11)
    at process.emit (/node_modules/source-map-support/source-map-support.js:435:16)
    at process._fatalException (bootstrap_node.js:391:26)

adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request May 3, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5854'
REST Spec PR Author 'ankurkhemani'
REST Spec PR Last commit
@ankurkhemani
Copy link
Contributor Author

//cc @mnltejaswini @djyou @sajayantony

@ankurkhemani ankurkhemani force-pushed the ankheman/v_19_05_GA_swagger branch 2 times, most recently from 9ce56f2 to 9f4f2fd Compare May 7, 2019 02:43
@sergey-shandar sergey-shandar added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label May 10, 2019
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request May 15, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5854'
REST Spec PR Author 'ankurkhemani'
REST Spec PR Last commit
@ravbhatnagar
Copy link
Contributor

@ankurkhemani - The new api-version does not contain many of the APis in the previous API version. Not sure why is this. Typically, all older APis are present in the latest api-version along with any additions or updates to existing. That way customer can move to the latest api-version and use all the functionality offered by your service. Please feel free to schedule a 30 min sync to close on this review quicker.

@ankurkhemani
Copy link
Contributor Author

@ravbhatnagar Those must be the Tasks APIs which are not part of this API version?
// cc @mnltejaswini

Anyways, I will schedule a meeting to review these changes.

@ankurkhemani
Copy link
Contributor Author

@ravbhatnagar Can you help me understand why is the CI failing for python and ruby sdk here?

@Azure Azure deleted a comment from adxsdknet May 21, 2019
@dsgouda
Copy link
Contributor

dsgouda commented May 21, 2019

startbuild

@ankurkhemani
Copy link
Contributor Author

@dsgouda @ravbhatnagar Can you please help me understand the CI fail for ruby sdk? Also, not sure why the PR for python-sdk and ruby-sdk is not generated?

@dsgouda
Copy link
Contributor

dsgouda commented May 21, 2019

@sergey-shandar please take a look

adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request May 22, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5854'
REST Spec PR Author 'ankurkhemani'
REST Spec PR Last commit
@adxsdknet
Copy link

adxsdknet commented May 22, 2019

Automation for azure-sdk-for-net

A PR has been created for you:
Azure/azure-sdk-for-net#6370
.NET SDK Commits:
adxsdknet/azure-sdk-for-net@604a9e4
adxsdknet/azure-sdk-for-net@e28efc5

@ravbhatnagar
Copy link
Contributor

ravbhatnagar commented May 22, 2019

3 changes in this new api-version

  1. PUT behavior changed to replace semantics for all resource types.
  2. policies on registry was being set using actions. In this api-version, its a property on the registry. Discussed cross api-version scenarios. The outstanding action item on ACR team is to block update of policies on a registry from an older api-version if it was created using the newer api version.
  3. policy action APis are being deprecated and hence dont exist in this new api-version. They will be supported to 12 months in the old api-version.
    another feedback was to make retentionPolicy status an enum
    Signing off from ARM side since the team has agreed to make the recommended changes.

@ravbhatnagar ravbhatnagar 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 ARMReviewMeetingRequired WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels May 22, 2019
@ankurkhemani
Copy link
Contributor Author

@ravbhatnagar @sergey-shandar As discussed, I have added the enum for retention policy status. Also, created a work item #4672516 to track cross-api scenarios as mentioned by @ravbhatnagar

Can you please help me with the CI here and get this PR merged? Also need to generate the python sdk. Thanks!

adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request May 24, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5854'
REST Spec PR Author 'ankurkhemani'
REST Spec PR Last commit
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.

7 participants