{Compute} Migrate util function to aaz-based implementation#32849
{Compute} Migrate util function to aaz-based implementation#32849william051200 wants to merge 2 commits intoAzure:devfrom
Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
|
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>
|
There was a problem hiding this comment.
Pull request overview
This PR migrates a SQL VM command module utility (create_ama_and_dcra) from using the Compute management SDK VM getter to an AAZ-based VM getter, aligning the az sql vm update implementation with the ongoing AAZ migration.
Changes:
- Replace
azure.cli.command_modules.vm.custom.get_vmusage withget_vm_by_aaz. - Update VM location retrieval to work with the AAZ show output shape (dict-like result).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| amainstall = build_ama_install_resource( | ||
| sql_virtual_machine_name, vm.location) | ||
| vm = get_vm_by_aaz(cmd, resource_group_name, sql_virtual_machine_name, 'instanceView') | ||
| amainstall = build_ama_install_resource(sql_virtual_machine_name, vm.get('location')) |
There was a problem hiding this comment.
get_vm_by_aaz returns a deserialized dict; using vm.get('location') will silently pass None into the ARM template if the key is missing (leading to a later deployment error that’s harder to diagnose). Prefer failing fast with vm['location'] or explicitly validating and raising a clear error. This matches existing usage patterns in azure/cli/command_modules/vm/custom.py where callers do vm['location'] after get_vm_by_aaz (e.g., around vm/custom.py:1292-1307).
| amainstall = build_ama_install_resource(sql_virtual_machine_name, vm.get('location')) | |
| amainstall = build_ama_install_resource(sql_virtual_machine_name, vm['location']) |
| 'instanceView') | ||
| amainstall = build_ama_install_resource( | ||
| sql_virtual_machine_name, vm.location) | ||
| vm = get_vm_by_aaz(cmd, resource_group_name, sql_virtual_machine_name, 'instanceView') |
There was a problem hiding this comment.
Switching from get_vm (management SDK) to get_vm_by_aaz changes the underlying VM GET to AAZ’s API version (currently 2025-04-01). The existing sqlvm scenario test recording test_sqlvm_mgmt_assessment.yaml includes a VM GET at api-version=2024-11-01, so playback is likely to break unless recordings are re-generated (or the call is adjusted to keep the previous api-version).
3 participants
Related command
az sql vm updateDescription
Migration from mgmt.compute to aaz-based
Testing Guide
History Notes
This 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.