Dev storagecache microsoft.storage cache 2023 03 01 preview#19724
Conversation
…2-05-01 to version 2022-11-01-preview
|
Hi, @brpanask Thanks for your PR. I am workflow bot for review process. Here are some small tips. Any feedback about review process or workflow bot, pls contact swagger and tools team. vscswagger@microsoft.com |
openapi-workflow-bot
bot
added
ARMReview
WaitForARMFeedback
|
Hi, @brpanask your PR are labelled with WaitForARMFeedback. A notification email will be sent out shortly afterwards to notify ARM review board(armapireview@microsoft.com). |
Swagger Generation Artifacts
|
|
Hi @brpanask, Your PR has some issues. Please fix the CI sequentially by following the order of
|
Swagger Generation Artifacts
|
Generated ApiView
|
|
/azp run unifiedPipeline |
|
No pipelines are associated with this pull request. |
|
/azp run unifiedPipeline |
|
No pipelines are associated with this pull request. |
|
/azp run unifiedPipeline |
|
No pipelines are associated with this pull request. |
brpanask
force-pushed
the
dev-storagecache-Microsoft.StorageCache-2022-11-01-preview
branch
from
2548b0f to
fcf4ba3
Compare
TimLovellSmith
added
ARMSignedOff
openapi-workflow-bot
bot
added
the
WaitForARMFeedback
|
LGTM now |
|
This ARMSignedOff PR still needs needs Breaking Changes Approved Label. I guess, that's why, automation added WaitForARMFeedback label again. Just FYI, @TimLovellSmith, @JeffreyRichter, @brpanask. |
visingla-ms
removed
the
WaitForARMFeedback
|
@dw511214992 For the additional properties warnings, I don't know what it needed. It has x-ms-flatten there. Similar to our private preview version of this and our storagecache.json file. The Patch Identity and Sku properties aren't modifiable. So I can't add them to the patch body since its not taking in readonly or createonly values. I just made the Patch body separate from the PUT body to fix other errors saying Location can't be in the Patch body. The Long Running Response Statue code must have 200 and 204 for delete. I have that so I'm not sure why this is an issue. |
|
There are many breaking changes shown here: https://github.com/Azure/azure-rest-api-specs/pull/19724/checks?check_run_id=10940083018 |
@JeffreyRichter Here is an early comment I left with the details and the PRs with the breaking changes approvals. For breaking changes, the list in the checks for this api are inherited from the changes that are merged into this api version. I'm going through the new comments now and doing what I can to work through the comments without additional breaking changes. The only one that should show up is the change from Cancelled to Canceled to match the ARM spec for provisioningState in stroagecache.json. It wasn't part of the codes enum so it can never be returned. I'll go through and point out the new ones which I know are Cancelled to Canceled and just a few others that just showed up. I'll still fill out a breaking change form and attend the meeting on Monday. |
2022-09-01-preview didn't need breaking change approval. The check passed and in it, it just noted the new field in the new api version and that a ref to a string was replaced with a string which fixed an s360 where x-ms-secret wasn't understood when applied to the ref, but did work directly on the string type.
|
The 2023-01-01 GA version just received breaking change approval 2 weeks ago.
The 2023-01-01 GA version had breaking changes approval 2 weeks ago and added
|
|
@JeffreyRichter 1007 - RemovedClientParameter | The new version is missing a client parameter that was found in the old version. Was 'ResourceGroupParameter' removed or renamed? This was due to a comment during review to use common-types instead of older types we originally had with the same content for the parameter. 1019 - RemovedEnumValue | The new version is removing enum value(s) 'Cancelled' from the old version. 1025 - RequiredStatusChange The 'required' status changed from the old version('True') to the new version('False'). 1027 - DefaultValueChanged The new version has a different default value than the previous one. |
mikekistler
added
the
Approved-BreakingChange
dw511214992
added
the
Approved-OkToMerge
| "400": { | ||
| "description": "The subnets provided do not meet the requirements for AML file system create.", | ||
| "schema": { | ||
| "$ref": "#/definitions/AmlFilesystemCheckSubnetError" | ||
| }, | ||
| "x-ms-error-response": true | ||
| }, | ||
| "default": { | ||
| "description": "Error response describing why the operation failed.", | ||
| "schema": { | ||
| "$ref": "storagecache.json#/definitions/CloudError" | ||
| } | ||
| } |
There was a problem hiding this comment.
Please note that even when this does not trigger a Linter error, this definition is not approparate for ARM.
- Use "default" if possible.
- If this error response is different from the "default", you can extend
CloudError, but you should still follow Error Response - Other cases is only allowed for backward compatibility concern. (but yours is new and in preview)
) * Adds base for updating Microsoft.StorageCache from version stable/2022-05-01 to version 2022-11-01-preview * Updates readme * Updates API version in new specs and examples * Baseline Approved 2023-01-01 GA version * Renaming old unused 2022-11-01-preview to 2023-03-01-preview * Merge Approved changes from Private Preview 2022-09-01-preview * Merge Approved Private Preview 2021-11-01-preview into new amlfilesystem.json file * undo customwords * Add separate PATCH for amlfs * Updates for spellcheck * Update API Version Parameter to common types. * Fix api version ref double quote error. * Use common parameters, move zones in aml, fix user/group example * Address comments for TrackedResource, descriptions, defaults * Fix validation error requiring Canceled state and location header for aml. * Updating to remove 202 from aml put
12 participants
This takes the amlfilesystem content from the 2021-11-01-preview api for storagecache Microsoft.StorageCache and adds the new public preview api 2023-03-01-preview version.
This 2023-03-01-preview version is based on storagecache 2022-05-01 version and the previously approved 2021-11-01-preview version's amlfilesystem content.
The amlfilesystem is a new resource that is currently in private preview and this api version will move it to public preview.
The only changes from the preview version of amlfilesystem content was the move to use a separate amlfilesystem.json file instead of adding it to the existing storagecache.json and to remove the management subnet that was in the private preview.
Adding this comment from below to make it more visible.
One question is on the comments to storagecache.json. That file has been that way for a few years and I think just about any change to that will be a breaking change against the existing service.
I squashed the fixes and made sure that the baseline was applied first which as from the 2022-05-01 GA version.
Then I added the approved GA 2023-01-01 version that is not merged yet as the next base.
After that the only real changes I applied to storagecache.json are from an approved 2022-09-01-preview spec in the private preview repo. That just included a new scalingFactor field.
The main content is the amlfilesystem.json. This was a new resource from the 2021-11-01-preview spec in the private repo.
It wasn't directly ported over from there because I made it its own file for public preview to help simplify reviews between existing content and new content.
stroagecache.json = GA Merged 2022-05-01 + GA Approved 2023-03-01 + Private Preview 2022-09-01-preview
amlfilesystem.json = Private Preview 2021-11-01-preview
This change for 2023-03-01-preview is the public preview of the 2 private previews for both storagecache and amlfilesystem.
Private Preview 2022-09-01-preview Pull Request
https://github.com/Azure/azure-rest-api-specs-pr/pull/7982
Private Preview 2021-11-01-preview Pull Request
https://github.com/Azure/azure-rest-api-specs-pr/pull/2097
Approved, not merged until RP rollout 2023-01-01
#22135
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Changelog
Add a changelog entry for this PR by answering the following questions:
Contribution checklist:
If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist
Otherwise your PR may be subject to ARM review requirements. Complete the following:
Check this box if any of the following apply to the PR so that the label "ARMReview" and "WaitForARMFeedback" will be added by bot to kick off ARM API Review. Missing to check this box in the following scenario may result in delays to the ARM manifest review and deployment.
-[ ] To review changes efficiently, ensure you are using OpenAPIHub to initialize the PR for adding a new version. More details, refer to the wiki.
Ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.
If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Breaking Change Review Checklist
If any of the following scenarios apply to the PR, request approval from the Breaking Change Review Board as defined in the Breaking Change Policy.
Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.
Please follow the link to find more details on PR review process.