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

Container Registries Data-plane API 07/2019 #6629

Merged
merged 6 commits into from
Aug 1, 2019

Conversation

estebanreyl
Copy link

@estebanreyl estebanreyl commented Jul 15, 2019

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. - Definitely tried it, I am not certain it worked correctly however as it output no errors and I know there are some warnings from other validation tools.

As a note a few linting issues could not be resolved as they are intrinsic in handling compatibility with docker's own expectations for registry API's I will also be following up in email as described in https://github.com/Azure/adx-documentation-pr/wiki/Swagger-Validation-Errors-Suppression

Preview Release for ACR data-plane API specification
This should be a fully functional release to address: #6457 .

Changes:

  • Added Put and Delete operations for v2//manifests/
  • Added oauth2 paths
  • Added support for V2 Manifests
  • Added improved and valid example files
  • Added appropriate security and host definitions

Bugfixes:

  • Cleaned up the entire swagger file re-using parameters and definition references
  • Fixed multiple improper response codes and incorrect parameters/return values

All paths have been verified and the generated client has been tested as well in C#. This should ideally lead to a release of a fully functioning .NET SDK.

As an edit we have decided to additionally add support for Go in this preview of our SDK

@kurtzeborn kurtzeborn added Container Registry Mgmt This issue is related to a management-plane library. labels Jul 15, 2019
@azuresdkci azuresdkci requested a review from erich-wang July 15, 2019 23:25
@AutorestCI
Copy link

AutorestCI commented Jul 15, 2019

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@erich-wang erich-wang added DataPlane and removed Mgmt This issue is related to a management-plane library. labels Jul 16, 2019
@erich-wang erich-wang added APIStewardshipBoard-ReviewRequested This should be reviewed by the Azure API Stewardship team in partnership with the service team. and removed APIStewardshipBoard-ReviewRequested This should be reviewed by the Azure API Stewardship team in partnership with the service team. labels Jul 16, 2019
@AutorestCI
Copy link

AutorestCI commented Jul 16, 2019

Automation for azure-sdk-for-go

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

@AutorestCI
Copy link

AutorestCI commented Jul 16, 2019

Automation for azure-sdk-for-python

Nothing to generate for azure-sdk-for-python

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 22, 2019

SDK Automation [Logs] (Generated from baf9fdd, Iteration 3)

Warning .NET: Azure/azure-sdk-for-net [Logs] [Diff]
  • No packages generated.
Failed Go: Azure/azure-sdk-for-go [Logs]
  • No packages generated.

@erich-wang erich-wang added the APIStewardshipBoard-ReviewRequested This should be reviewed by the Azure API Stewardship team in partnership with the service team. label Jul 25, 2019
@erich-wang
Copy link
Member

@estebanreyl, sorry for late response. To speed up review, could you reorganize the PR: the first commit is copied from previous version, it is 2018-08-10 in your case, then make change based on first commit, in this way the reviewer could ignore APIs which are same as previous version and only focus on new/changed APIs, thanks.

@estebanreyl
Copy link
Author

estebanreyl commented Jul 26, 2019

@erich-wang Do you mean as in just adding the original file (copy pasted to its new location) as the first commit so one can do a diff from there on? I could try rebasing from there on and add that in if that will help at all? Nonetheless this swagger is based on a fork of a fork which was originally quite distant from the original. Ill try that out this afternoon and update. Is reforming this branch to this : https://github.com/AzureCR/azure-rest-api-specs/tree/estebanreyl/data-plane-rebased what you want? I can replace this branch accordingly if that's the case

@johanste johanste removed the APIStewardshipBoard-ReviewRequested This should be reviewed by the Azure API Stewardship team in partnership with the service team. label Jul 26, 2019
@johanste
Copy link
Member

This API is already in production. No need for a Azure REST API review board to review.

@erich-wang
Copy link
Member

@estebanreyl , one question from @johanste:

And to verify, the casing changes for the model types are intentional?

@estebanreyl
Copy link
Author

@estebanreyl , one question from @johanste:
And to verify, the casing changes for the model types are intentional?

Do you mean in the parameters area? I think that was just an effort to make them match the same style as those under definitions. I do realize I missed a couple however. Should those all be lowercased? I am happy to change them if that is the case.

@erich-wang
Copy link
Member

@estebanreyl , one question from @johanste:
And to verify, the casing changes for the model types are intentional?

Do you mean in the parameters area? I think that was just an effort to make them match the same style as those under definitions. I do realize I missed a couple however. Should those all be lowercased? I am happy to change them if that is the case.

@johanste to response

@johanste
Copy link
Member

If you are going to change things for consistency, I'd prefer it to end up being 100% consistent. But it also looks like the parameters were not referenced (at least not from this file) in the previous version of the swagger. Either way, for a preview, this is not a strict requirement - especially since it is changing parameter reference model names (as opposed to the name of the actual parameter changing)

@estebanreyl
Copy link
Author

@johanste I'll make a note and include it in the upcoming next release or fix it now if there is anything else stopping this from going through. Is there anything else I should look into?

@estebanreyl
Copy link
Author

@johanste , @erich-wang Any chance we get an update on what we are missing if anything here so we can proceed with this one?

@johanste
Copy link
Member

johanste commented Aug 1, 2019

I'm good. @erich-wang ?

@erich-wang erich-wang merged commit 7cfa94a into Azure:master Aug 1, 2019
nschonni added a commit to nschonni/azure-rest-api-specs that referenced this pull request Aug 1, 2019
Fix spelling issues added in Azure#6629 from "example" values in spec
@nschonni nschonni mentioned this pull request Aug 1, 2019
mmyyrroonn pushed a commit that referenced this pull request Aug 1, 2019
Fix spelling issues added in #6629 from "example" values in spec
@estebanreyl estebanreyl mentioned this pull request Aug 30, 2019
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants