-
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 STG 87 #22451
Storage STG 87 #22451
Conversation
Hi, @seanmcc-msft 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 |
Swagger Validation Report
|
compared swaggers (via Oad v0.10.4)] | new version | base version |
---|---|---|
DataLakeStorage.json | 2021-06-08(61b7e4a) | 2021-06-08(main) |
Rule | Message |
---|---|
1041 - AddedPropertyInResponse |
The new version has a new property 'EncryptionContext' in response that was not found in the old version. New: Azure.Storage.Files.DataLake/preview/2021-06-08/DataLakeStorage.json#L3280:7 Old: Azure.Storage.Files.DataLake/preview/2021-06-08/DataLakeStorage.json#L3268:7 |
1043 - AddingOptionalParameter |
The optional parameter 'x-ms-encryption-context' was added in the new version. New: Azure.Storage.Files.DataLake/preview/2021-06-08/DataLakeStorage.json#L613:9 |
️❌
Breaking Change(Cross-Version): 80 Errors, 0 Warnings failed [Detail]
compared swaggers (via Oad v0.10.4)] | new version | base version |
---|---|---|
file.json | 2022-11-02(61b7e4a) | 2021-12-02(main) |
The following breaking changes are detected by comparison with the latest preview version:
Only 30 items are listed, please refer to log for more details.
️️✔️
CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️
LintDiff: 3 Warnings warning [Detail]
compared tags (via openapi-validator v2.0.0) | new version | base version |
---|---|---|
package-2022-11 | package-2022-11(61b7e4a) | default(main) |
package-2021-06 | package-2021-06(61b7e4a) | package-2021-06(main) |
[must fix]The following errors/warnings are introduced by current PR:
Rule | Message | Related RPC [For API reviewers] |
---|---|---|
Property should have a description. Location: Azure.Storage.Files.DataLake/preview/2021-06-08/DataLakeStorage.json#L3317 |
||
Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.FileStorage/preview/2022-11-02/file.json#L7363 |
||
Booleans properties are not descriptive in all cases and can make them to use, evaluate whether is makes sense to keep the property as boolean or turn it into an enum. Location: Microsoft.FileStorage/preview/2022-11-02/file.json#L7371 |
The following errors/warnings exist before current PR submission:
Only 30 items are listed, please refer to log for more details.
️️✔️
Avocado succeeded [Detail] [Expand]
Validation passes for Avocado.
️️✔️
ApiReadinessCheck succeeded [Detail] [Expand]
️⚠️
~[Staging] ServiceAPIReadinessTest: 0 Warnings warning [Detail]
API Test is not triggered due to precheck failure. Check pipeline log for details.
️❌
SwaggerAPIView: 0 Errors, 0 Warnings failed [Detail]
️️✔️
CadlAPIView succeeded [Detail] [Expand]
️️✔️
ModelValidation succeeded [Detail] [Expand]
Validation passes for ModelValidation.
️️✔️
SemanticValidation succeeded [Detail] [Expand]
Validation passes for SemanticValidation.
️️✔️
PoliCheck succeeded [Detail] [Expand]
Validation passed for PoliCheck.
️️✔️
PrettierCheck succeeded [Detail] [Expand]
Validation passes for PrettierCheck.
️️✔️
SpellCheck succeeded [Detail] [Expand]
Validation passes for SpellCheck.
️️✔️
Lint(RPaaS) succeeded [Detail] [Expand]
Validation passes for Lint(RPaaS).
️️✔️
CadlValidation succeeded [Detail] [Expand]
Validation passes for CadlValidation.
️️✔️
PR Summary succeeded [Detail] [Expand]
Validation passes for Summary.
Swagger pipeline restarted successfully, please wait for status update in this comment. |
Swagger pipeline started successfully. If there is ApiView generated, it will be updated in this comment. |
Hi @seanmcc-msft, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review. |
Hi, @seanmcc-msft, For review efficiency consideration, when creating a new api version, it is required to place API specs of the base version in the first commit, and push new version updates into successive commits. You can use OpenAPIHub to initialize the PR for adding a new version. For more details refer to the wiki. Or you could onboard API spec pipeline |
@tg-msft, could you take a look? |
DataLake was easy to diff in the PR and I used https://portal.azure-devex-tools.com/tools/diff/eyJzb3VyY2VVcmwiOiJodHRwczovL2dpdGh1Yi5jb20vQXp1cmUvYXp1cmUtcmVzdC1hcGktc3BlY3MvYmxvYi9tYWluL3NwZWNpZmljYXRpb24vc3RvcmFnZS9kYXRhLXBsYW5lL01pY3Jvc29mdC5GaWxlU3RvcmFnZS9wcmV2aWV3LzIwMjEtMTItMDIvZmlsZS5qc29uIiwidGFyZ2V0VXJsIjoiaHR0cHM6Ly9naXRodWIuY29tL0F6dXJlL2F6dXJlLXJlc3QtYXBpLXNwZWNzL2Jsb2IvNjFiN2U0YWQ5MWQ0NGQzZGFjZmNhZWU2YzVmNGNmZDAwODAyMmE3Ni9zcGVjaWZpY2F0aW9uL3N0b3JhZ2UvZGF0YS1wbGFuZS9NaWNyb3NvZnQuRmlsZVN0b3JhZ2UvcHJldmlldy8yMDIyLTExLTAyL2ZpbGUuanNvbiJ9 to diff Files. None of the changes are breaking so I added the Approved Breaking Change label. |
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.
* Added new file version (Azure#21704) * Added HNS Encryption Context (Azure#21693) * Added x-ms-file-request-intent to File and Directory operations (Azure#21705) * Trailing Dot (Azure#21781) * Add file request intent to comp=rename for dir&files (Azure#22420) --------- Co-authored-by: vincenttran-msft <101599632+vincenttran-msft@users.noreply.github.com>
This is not a breaking change
The Encryption Context feature in the Data Lake swagger was implemented and enabled about a year ago, we are just adding SDK support now.
This is not a breaking change