-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[Storage] Add object replication policy #8864
Conversation
Azure Pipelines successfully started running 1 pipeline(s). |
azure-sdk-for-java - Release
|
azure-sdk-for-go - Release
|
azure-sdk-for-python - Release
|
azure-cli-extensions - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
azure-sdk-for-js - Release
|
azure-sdk-for-net - Release
|
Can one of the admins verify this patch? |
"description": "Optional. Filters the results to replicate only blobs whose names begin with the specified prefix." | ||
}, | ||
"minCreationTime": { | ||
"type": "string", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be datetime format. Using string format will make we need to add a lot of code to handle to format convert in Powershell (and other user of SDK), which easy to take bugs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using datetime format will break the server side. Server side could support datetime format in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would it break server side? It would still be serialized as a string in JSON.
In reply to: 401773509 [](ancestors = 401773509)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be good to get this fixed up front so we generate SDKs correctly from the beginning.
In reply to: 403179698 [](ancestors = 403179698,401773509)
For the .NET SDK failure, it caused by a before swager change in #8595. I have raised a PR to fix the unit test in .NET SDK Azure/azure-sdk-for-net#10867, We should rerun the .NET SDK test after the PR is in. [Update on 3/30/2020] The .NET SDK PR is in, and the .NET SDK check pass now. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Azure Pipelines successfully started running 1 pipeline(s). |
Please add ARM team for review if needed. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks identical to the previously approved private PR (except for the one date-time format change). Signed off from ARM side. Please clear up the date-time format issue before merging, though.
* [Storage] Add object replication policy * Fix spellcheck and prettier check
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:
ARM API Review Checklist
Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.
Please follow the link to find more details on API review process.
#https://github.com/Azure/azure-rest-api-specs-pr/pull/965/files