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

Improvements/Fixes for ACR SDK #6457

Closed
estebanreyl opened this issue Jun 26, 2019 · 12 comments
Closed

Improvements/Fixes for ACR SDK #6457

estebanreyl opened this issue Jun 26, 2019 · 12 comments
Assignees

Comments

@estebanreyl
Copy link

estebanreyl commented Jun 26, 2019

We have been notified that some features are not currently supported/working as expected in the Azure Container Registries SDK for .NET generated from our provided swagger in particular :

1.	Pagination for getting repositories and tags is hard to use
2.	v2 manifest is not supported
3.	PUT manifest is not supported

Other API calls also require validation/testing. This is a small issue tracker for this work item. Will update as more progress is done.

@graypj
Copy link

graypj commented Jun 26, 2019

Adding more details:

  1. Pagination is a response header and current swagger is not parsing in in responses.
Link: </v2/_catalog?last=azcopy&n=1&orderby=>; rel=\"next\"
  1. /v2/{name}/manifests/{reference} only supports Get method with default header. Put and Delete method is not supported yet. Also application/vnd.docker.distribution.manifest.v2+json mediaType is required do get manifest in v2 schema.

  2. Tag deletion is expecting 204 for success code but it's actually 202.

  3. /oauth2/exchange is not supported

  4. /oauth2/token is not supported.

@graypj
Copy link

graypj commented Jun 26, 2019

  1. Tag deletion is expecting 204 for success code but it's actually 202.

This is currently a blocking issue for us. It would be great if this can be prioritized.

@estebanreyl
Copy link
Author

estebanreyl commented Jun 26, 2019

@graypj Will take a look at that immediately. Is the issue just that the success code is incorrect?

@graypj
Copy link

graypj commented Jun 26, 2019

That's what I found. I haven't tested other scenarios.

@estebanreyl
Copy link
Author

To update. I have replicated issue 3. Also confirmed 202 is the only expected correct status. The same issue exists for patch which also relies on 204 but should be 201. We do have a more updated swagger that does include Put for /v2/{name}/manifests/{reference} and addresses these last couple of issues at estebanreyl@4f95280 . I am currently working on top of it to address the other concerns and validate the generation of the SDK. You can probably look forward to a lot of progress tomorrow as it took me some time to understand how this all worked. Nonetheless if the 202 issue was the blocking component we could build the sdk using autorest immediately before proceeding with getting everything upstream. @graypj How are you using the SDK, are you manually building it or using the preview nugget package ?

@graypj
Copy link

graypj commented Jun 27, 2019

We are currently using the preview nugget package. Can that be updated as well when these features are added?

@estebanreyl
Copy link
Author

Absolutely, Ill begin asking how this can be carried out.

@estebanreyl
Copy link
Author

estebanreyl commented Jun 28, 2019

To update on the previous point it seems as soon as this goes through here we can quickly get the preview nugget package out. As an additional update, as of commit estebanreyl@4999232 I have at least in theory (Still working on testing to make sure everything is indeed running smoothly) implemented items 1,2,3 along with fixing other portions of the swagger that seem to be inconsistent with our api. Still need to do a lot of testing and oauth2 is pending. Will update as appropiate.

@michaeljqzq
Copy link
Contributor

Added @erich-wang since it is .NET SDK related issue

@erich-wang
Copy link
Member

Please refer to the process (visible only to Microsoft employee) if you need to generate .net management SDK from swagger spec

@estebanreyl
Copy link
Author

https://github.com/AzureCR/azure-rest-api-specs/tree/estebanreyl/dot_net_sdk Is now housing the most updated work in progress of this. Oauth2 paths have been added though the entire flow (As a result of Swagger 2 limitations) will be custom built in C# for that particular sdk. The same will have to be done for others generated from this swagger. Most paths have been verified to work. Still have to look at distinct versions of manifests though.

@estebanreyl
Copy link
Author

To update as of AzureCR@5b84330 all discussed features have been implemented and tested. I will begin the process of making sure everything is up to standards as specified here . Will hopefully get through that quickly and PR.

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

No branches or pull requests

4 participants