{Compute} az restore-point: Migrate to AAZ#32391
Conversation
️✔️AzureCLI-FullTest
|
|
Hi @cxznmhdcxz, |
️✔️AzureCLI-BreakingChangeTest
|
microsoft-github-policy-service
bot
requested review from
yanzhudd,
yonzhan and
zhoxing-ms
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
added
the
Compute
cxznmhdcxz
force-pushed
the
restore-point-migrate
branch
from
8387cfc to
cfe8fba
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the restore-point commands from SDK-based implementation to AAZ (Auto-generated Azure CLI) framework, updating to API version 2024-11-01 from 2022-08-01.
- Migrates
restore-point create,show, andwaitcommands to use AAZ-generated code - Updates API version from 2022-08-01 to 2024-11-01
- Removes direct SDK client factory dependency (
cf_restore_point) - Adds
--size Standard_B2msparameter to a test VM creation for more reliable test execution
Reviewed Changes
Copilot reviewed 10 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
custom.py |
Refactored restore_point_create and restore_point_show to use AAZ command pattern instead of SDK client calls |
commands.py |
Removed cf_restore_point client factory import and usage from restore-point command group |
_client_factory.py |
Removed cf_restore_point factory function that is no longer needed |
aaz/latest/restore_point/_create.py |
Added auto-generated AAZ implementation for restore-point create command |
aaz/latest/restore_point/_show.py |
Added auto-generated AAZ implementation for restore-point show command |
aaz/latest/restore_point/_wait.py |
Added auto-generated AAZ implementation for restore-point wait command |
aaz/latest/restore_point/_delete.py |
Updated API version and LRO polling strategy to use location-based polling |
aaz/latest/restore_point/__init__.py |
Added imports for new AAZ command implementations |
aaz/latest/restore_point/__cmd_group.py |
Added comment referencing swagger API spec change |
test_vm_commands.py |
Added explicit VM size to test for better reliability |
Comments suppressed due to low confidence (1)
src/azure-cli/azure/cli/command_modules/vm/custom.py:5825
- The parameter key 'excludeDisks' uses camelCase, but according to the AAZ command schema shown in _create.py, the expected argument name is 'exclude_disks' (snake_case). This inconsistency will cause the exclude_disks parameter to not be properly passed to the AAZ command.
if exclude_disks is not None:
parameters['excludeDisks'] = []
for disk in exclude_disks:
parameters['excludeDisks'].append({'id': disk})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from .aaz.latest.restore_point import Create | ||
| return Create(cli_ctx=cmd.cli_ctx)(command_args=parameters) |
There was a problem hiding this comment.
The implementation removes all custom parameter processing logic (lines 5822-5944 in the original code) that handled exclude_disks, source_restore_point, consistency_mode, storage_profile configuration, and encryption settings. The AAZ Create command expects these parameters in a specific format that differs from how they were being constructed. The parameters dictionary should be properly mapped to match AAZ command's expected argument names and structure.
| args = { | ||
| 'resource_group': resource_group_name, | ||
| 'restore_point_collection_name': restore_point_collection_name, | ||
| 'restore_point_name': restore_point_name, | ||
| 'expand': expand | ||
| } |
There was a problem hiding this comment.
The function parameter 'instance_view' is accepted but never used. It's only used to conditionally set expand to 'instanceView', which could overwrite the passed expand parameter. Consider removing the instance_view parameter if it's deprecated, or handle both parameters more explicitly to avoid confusion.
| 'no_wait': no_wait | ||
| } | ||
| if exclude_disks is not None: | ||
| parameters['excludeDisks'] = [] |
There was a problem hiding this comment.
I think the name of parameters may should be snake case
Related command
az restore-point create/show/deleteDescription
aaz Azure/aaz#895
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.