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

Update Storage swagger #4981

Merged
merged 7 commits into from
Aug 15, 2019
Merged

Update Storage swagger #4981

merged 7 commits into from
Aug 15, 2019

Conversation

tg-msft
Copy link
Member

@tg-msft tg-msft commented Aug 14, 2019

There are two commits in this PR that might be easier to review individually.

The first changes to the official versions of the Storage swagger and adds README.md transformations address any deltas. I've regenerated and there was only one code change I couldn't trace back to the swagger which I'll call out below.

The second commit updates blobs to support STG69 features. This will require the feature crew to take over this PR and update any calls to the protocol layer that added additional parameters (I assume null will be fine to get started).

There are no (meaningful) code changes except the attribute in
BlobHierarchyListSegment that I can't trace to the swagger
Note that additional work will have to be done to patch our higher level code to
pass new parameters to existing APIs
@sima-zhu
Copy link
Contributor

#4980

I created an issue for missing error constant in StorageErrorCode. The issue only exists in Storage queue. Could you please take a look? It might something wrong with the service or missing the error code in swagger.
I have a PR manually added them in.

@alzimmermsft alzimmermsft self-assigned this Aug 14, 2019
``` yaml
directive:
- from: swagger-document
where: $["x-ms-paths"]["/{containerName}/{blob}?comp=copy&sync"]
Copy link
Member

Choose a reason for hiding this comment

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

Can't this be combined with the comp=copy version directly above this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not aware of a way to do that.

Copy link
Member

Choose a reason for hiding this comment

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

I thought the where clauses were json path syntax and if so I'd expect something like /{containerName}/{blob}?comp=copy* to work. Although I'm no expert in json path or these autorest directives so perhaps that is just wishful thinking.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe? The only documentation I've found doesn't mention that explicitly, but it does look like JSON Path syntax. I don't think you can use a wildcard * inside a string like that, but there's probably a way using those arcane JSON Path filter expressions. I don't want to block progress on getting this work done though so we can explore that after this PR.

@alzimmermsft alzimmermsft removed their request for review August 15, 2019 01:17
@tg-msft
Copy link
Member Author

tg-msft commented Aug 15, 2019

Nice, these changes look great.

@rickle-msft -- mind taking a look at Alan's commits in particular to CR the way he's changing the clients to call the new 2019-02-02 protocol layer?

@alzimmermsft
Copy link
Member

#5001 Calls out the changes that new features around Content-MD5 and x-ms-content-crc64 need to be handled as well as other potential changes that need to be handled.

@alzimmermsft
Copy link
Member

/azp run Azure.azure-sdk-for-java-mgmt (Build Java 7)

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@alzimmermsft
Copy link
Member

/azp run Azure.azure-sdk-for-java-mgmt

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@alzimmermsft alzimmermsft merged commit 99e76b4 into Azure:master Aug 15, 2019
@alzimmermsft alzimmermsft deleted the storage-swagger branch August 15, 2019 18:46
pull bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 15, 2019
* Move Storage to official swagger files

There are no (meaningful) code changes except the attribute in
BlobHierarchyListSegment that I can't trace to the swagger

* Upgrade blobs swagger to 2019-02-02

Note that additional work will have to be done to patch our higher level code to
pass new parameters to existing APIs

* Fixes to Swagger generation and updating to use new protocol layer

* Fixed appendBlock content-type, added SpotBug exclusions for autogen code

* Update README.md

* Fixed incorrect exclude
pull bot pushed a commit to test-repo-tih/azure-sdk-for-java that referenced this pull request Aug 15, 2019
* Move Storage to official swagger files

There are no (meaningful) code changes except the attribute in
BlobHierarchyListSegment that I can't trace to the swagger

* Upgrade blobs swagger to 2019-02-02

Note that additional work will have to be done to patch our higher level code to
pass new parameters to existing APIs

* Fixes to Swagger generation and updating to use new protocol layer

* Fixed appendBlock content-type, added SpotBug exclusions for autogen code

* Update README.md

* Fixed incorrect exclude
pull bot referenced this pull request in AzureSDKAutomation/azure-sdk-for-java Aug 15, 2019
* Move Storage to official swagger files

There are no (meaningful) code changes except the attribute in
BlobHierarchyListSegment that I can't trace to the swagger

* Upgrade blobs swagger to 2019-02-02

Note that additional work will have to be done to patch our higher level code to
pass new parameters to existing APIs

* Fixes to Swagger generation and updating to use new protocol layer

* Fixed appendBlock content-type, added SpotBug exclusions for autogen code

* Update README.md

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

Successfully merging this pull request may close these issues.

6 participants