-
Notifications
You must be signed in to change notification settings - Fork 5.3k
batch: the storageAccountId field is no longer required #21291
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
base: main
Are you sure you want to change the base?
Conversation
This needs to be Optional so that it can be `null` during an Update to remove this value The current Azure SDK outputs this as `*string` rather than `string` which is a bug in the SDK Generator: https://github.com/Azure/azure-sdk-for-go/blob/297f2c8b6d448f01b09fbc5c7fad393922c9ccda/services/batch/mgmt/2022-01-01/batch/models.go#L872 However since this needs to be `null` we should make this Optional
Hi, @tombuildsstuff 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. [email protected] |
Swagger Validation Report
|
compared swaggers (via Oad v0.10.1)] | new version | base version |
---|---|---|
BatchManagement.json | 2022-01-01(d8b02ea) | 2022-01-01(main) |
Rule | Message |
---|---|
1025 - RequiredStatusChange |
The 'required' status changed from the old version('False') to the new version('True'). New: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L2497:7 Old: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L2497:7 |
1025 - RequiredStatusChange |
The 'required' status changed from the old version('False') to the new version('True'). New: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L2685:7 Old: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L2688:7 |
️️✔️
Breaking Change(Cross-Version) succeeded [Detail] [Expand]
There are no breaking changes.
️️✔️
CredScan succeeded [Detail] [Expand]
There is no credential detected.
️⚠️
LintDiff: 0 Warnings warning [Detail]
compared tags (via openapi-validator v1.13.0) | new version | base version |
---|---|---|
package-2022-01 | package-2022-01(d8b02ea) | package-2022-01(main) |
The following errors/warnings exist before current PR submission:
Rule | Message |
---|---|
R4041 - XmsIdentifierValidation |
Missing identifier registryServers in array item property Location: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L4516 |
The child tracked resource, 'applications' with immediate parent 'BatchAccount', must have a list by immediate parent operation. Location: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L3091 |
|
The child tracked resource, 'versions' with immediate parent 'Application', must have a list by immediate parent operation. Location: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L3125 |
|
The child tracked resource, 'certificates' with immediate parent 'BatchAccount', must have a list by immediate parent operation. Location: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L3500 |
|
The child tracked resource, 'detectors' with immediate parent 'BatchAccount', must have a list by immediate parent operation. Location: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L3599 |
|
The child tracked resource, 'privateLinkResources' with immediate parent 'BatchAccount', must have a list by immediate parent operation. Location: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L3625 |
|
The child tracked resource, 'privateEndpointConnections' with immediate parent 'BatchAccount', must have a list by immediate parent operation. Location: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L3669 |
|
The child tracked resource, 'pools' with immediate parent 'BatchAccount', must have a list by immediate parent operation. Location: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L3793 |
️️✔️
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.
️❌
ModelValidation: 2 Errors, 0 Warnings failed [Detail]
Rule | Message |
---|---|
LRO_RESPONSE_HEADER |
Long running operation should return location or azure-AsyncOperation in header but not provided Url: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L198:22 ExampleUrl: stable/2022-01-01/examples/BatchAccountDelete.json |
LRO_RESPONSE_CODE |
Respond to the initial request of a long running operation, Patch/Post call must return 201 or 202, Delete call must return 202 or 204, Put call must return 202 or 201 or 200, but 204 being returned Url: Microsoft.Batch/stable/2022-01-01/BatchManagement.json#L1953:22 ExampleUrl: stable/2022-01-01/examples/PrivateEndpointConnectionUpdate.json |
️️✔️
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 Generation Artifacts
|
Generated ApiView
|
Thank you for your contribution tombuildsstuff! We will review the pull request and get back to you soon. |
Hi @tombuildsstuff, Your PR has some issues. Please fix the CI sequentially by following the order of
|
Trying to remove
|
Hi, @tombuildsstuff. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove |
Hi @tombuildsstuff - I'm from the Batch team and I can confirm that this property isn't nullable (at least from a REST API perspective). The way to remove auto-storage from a Batch account would be to set the entire Valid:
Invalid:
|
Hi, @tombuildsstuff. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove |
Waiting on the Service Team.. |
Hi, @tombuildsstuff. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove |
Next Steps to MergeNext steps that must be taken to merge this PR:
|
This needs to be Optional so that it can be
null
during an Update to remove this valueThe current Azure SDK outputs this as
*string
rather thanstring
which is a bug in the SDK Generator: https://github.com/Azure/azure-sdk-for-go/blob/297f2c8b6d448f01b09fbc5c7fad393922c9ccda/services/batch/mgmt/2022-01-01/batch/models.go#L872However since this needs to be
null
we should make this Optional