{AEM} Migrate commands calling Compute module to aaz-based implementation#9765
{AEM} Migrate commands calling Compute module to aaz-based implementation#9765william051200 wants to merge 8 commits intoAzure:mainfrom
Conversation
️✔️Azure CLI Extensions Breaking Change Test
|
|
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 updates the aem extension to migrate VM/compute-related operations away from azure.mgmt.compute SDK usage toward AAZ-based command implementations, and bumps the extension version accordingly.
Changes:
- Bump aem extension version to 1.0.2 and add release notes for the migration.
- Replace VM get/update, VM extension create/delete, and disk show operations with AAZ/command-module implementations.
- Adjust VM data access from SDK model objects to dict-style payloads returned by AAZ/command operations.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/aem/setup.py |
Version bump to 1.0.2. |
src/aem/HISTORY.rst |
Adds 1.0.2 release notes describing the AAZ migration. |
src/aem/azext_aem/custom.py |
Migrates compute operations to AAZ/command-module calls and updates VM/extension/disk handling accordingly. |
src/aem/azext_aem/custom.py
Outdated
| }, | ||
| 'type': self._extension.get('name'), | ||
| 'type_handler_version': self._extension.get('version'), | ||
| 'no_wait': self.no_wait |
There was a problem hiding this comment.
no_wait is stored on the instance as self._no_wait, but this command args dict references self.no_wait, which is not defined anywhere in this class and will raise AttributeError at runtime. Use self._no_wait (or add a @property no_wait that returns _no_wait) consistently for AAZ/operation calls.
| 'no_wait': self.no_wait | |
| 'no_wait': self._no_wait |
src/aem/azext_aem/custom.py
Outdated
| }, | ||
| 'type': self._extension.get('name'), | ||
| 'type_handler_version': self._extension.get('version'), | ||
| 'no_wait': self.no_wait |
There was a problem hiding this comment.
Same self.no_wait issue as above: self.no_wait is not defined (only _no_wait exists), so this will crash when installing the old extension path. Use _no_wait or define a property.
src/aem/azext_aem/custom.py
Outdated
| 'resource_group': self._resource_group, | ||
| 'vm_extension_name': existing_ext['name'], | ||
| 'vm_name': self._vm['name'], | ||
| 'no_wait': self.no_wait |
There was a problem hiding this comment.
Same self.no_wait issue as above in delete: self.no_wait is not defined and will raise at runtime. Use self._no_wait (or a property) when passing no_wait into the AAZ delete command.
| 'no_wait': self.no_wait | |
| 'no_wait': self._no_wait |
src/aem/azext_aem/custom.py
Outdated
| command_args['mi_system_assigned'] = 'True' | ||
|
|
||
| poller = VMPatch(cli_ctx=self._cmd.cli_ctx)(command_args=command_args) | ||
| self._vm = LongRunningOperation(self._cmd.cli_ctx)(poller).result() |
There was a problem hiding this comment.
LongRunningOperation(cli_ctx)(poller) already waits and returns the final result; calling .result() on its return value is inconsistent with how LongRunningOperation is used elsewhere in this repo and will likely fail (e.g., if it returns a dict). Assign self._vm = LongRunningOperation(self._cmd.cli_ctx)(poller) instead.
| self._vm = LongRunningOperation(self._cmd.cli_ctx)(poller).result() | |
| self._vm = LongRunningOperation(self._cmd.cli_ctx)(poller) |
| 'vm_name': self._vm['name'], | ||
| } | ||
| if new_identity == IDENTITY_SYSTEM_ASSIGNED or new_identity == IDENTITY_SYSTEM_USER_ASSIGNED: | ||
| command_args['mi_system_assigned'] = 'True' |
There was a problem hiding this comment.
mi_system_assigned is a boolean-style AAZ/CLI argument in other command implementations, but here it is being set to the string 'True'. This can break argument parsing/serialization. Pass a real boolean (True) instead of a string.
| command_args['mi_system_assigned'] = 'True' | |
| command_args['mi_system_assigned'] = True |
Migration from mgmt.compute to aaz-based
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install wheel==0.30.0required)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.