-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Create Az CloudHsm Extension for 2025-03-31 #8862
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
Conversation
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
Hi @emmeliaAra, |
|
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>
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
CodeGen Tools Feedback CollectionThank you for using our CodeGen tool. We value your feedback, and we would like to know how we can improve our product. Please take a few minutes to fill our codegen survey |
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.
Pull Request Overview
Adds the CloudHSM extension for Azure CLI (API version 2025-03-31), including new commands for cluster management, backup/restore operations, and wait/show/delete actions.
- Introduces
backup start/showandrestore start/showcommands with LRO support. - Defines main
cloudhsmcommand group and subgroups forbackup,restore, andwait. - Adds basic README and HISTORY entries for the new extension.
Reviewed Changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| azext_cloudhsm/aaz/latest/cloudhsm/backup/_start.py | Defines az cloudhsm backup start command |
| azext_cloudhsm/aaz/latest/cloudhsm/backup/_show.py | Implements az cloudhsm backup show command |
| azext_cloudhsm/aaz/latest/cloudhsm/restore/_start.py | Defines az cloudhsm restore start command |
| azext_cloudhsm/aaz/latest/cloudhsm/restore/_show.py | Implements az cloudhsm restore show command |
| azext_cloudhsm/aaz/latest/cloudhsm/_wait.py | Implements az cloudhsm wait command |
| azext_cloudhsm/aaz/latest/cloudhsm/_show.py | Implements az cloudhsm show command |
| azext_cloudhsm/aaz/latest/cloudhsm/_delete.py | Implements az cloudhsm delete command |
| src/cloudhsm/README.md | Adds placeholder usage instructions |
| src/cloudhsm/HISTORY.rst | Adds initial release history |
Comments suppressed due to low confidence (1)
src/cloudhsm/azext_cloudhsm/aaz/latest/cloudhsm/backup/_start.py:14
- There are no existing CLI tests for
az cloudhsm backup start. Consider adding functional tests to validate LRO polling behavior and output deserialization.
@register_command(
| # define Arg Group "BackupRequestProperties" | ||
|
|
||
| _args_schema = cls._args_schema | ||
| _args_schema.blob_container_uri = AAZStrArg( |
Copilot
AI
Jun 13, 2025
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.
The --blob-container-uri argument is required for the backup command but required=True is missing. Please add required=True to ensure the CLI enforces this input.
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.
From the Swagger this property is required. however during flattening this property became optional. To enforce this property in the custom.py file I set args_schema_backup.blob_container_uri._required = True.
I did not modify the files generated automatically as per the guidelines. Do I update the start.py file or can we ignore this message ?
| pass | ||
|
|
||
|
|
Copilot
AI
Jun 13, 2025
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.
The on_202 callback is empty, so HTTP 202 responses will not populate or return any output. Consider handling in-progress status similar to on_200 or returning a meaningful placeholder.
| pass | |
| data = self.deserialize_http_content(session) | |
| self.ctx.set_var( | |
| "instance", | |
| data, | |
| schema_builder=self._build_schema_on_202 | |
| ) | |
| _schema_on_202 = None | |
| @classmethod | |
| def _build_schema_on_202(cls): | |
| if cls._schema_on_202 is not None: | |
| return cls._schema_on_202 | |
| cls._schema_on_202 = AAZObjectType() | |
| _schema_on_202 = cls._schema_on_202 | |
| _schema_on_202.properties = AAZObjectType() | |
| properties = cls._schema_on_202.properties | |
| properties.status = AAZStrType( | |
| serialized_name="status", | |
| nullable=True, | |
| flags={"read_only": True}, | |
| ) | |
| properties.message = AAZStrType( | |
| serialized_name="message", | |
| nullable=True, | |
| flags={"read_only": True}, | |
| ) | |
| return cls._schema_on_202 |
| return cls._schema_on_200 | ||
|
|
||
| def on_202(self, session): | ||
| pass |
Copilot
AI
Jun 13, 2025
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.
The on_202 callback is a no-op, so intermediate HTTP 202 statuses in the backup show command aren’t surfaced. You may want to mirror on_200 behavior or explicitly indicate a pending state.
| pass | |
| data = self.deserialize_http_content(session) | |
| self.ctx.set_var( | |
| "pending_status", | |
| { | |
| "message": "The backup operation is still in progress.", | |
| "details": data | |
| } | |
| ) |
|
src/cloudhsm/azext_cloudhsm/aaz/latest/cloudhsm/backup/_show.py
Outdated
Show resolved
Hide resolved
src/cloudhsm/azext_cloudhsm/aaz/latest/cloudhsm/restore/_show.py
Outdated
Show resolved
Hide resolved
|
|
||
| from knack.log import get_logger | ||
| from .aaz.latest.cloudhsm._create import Create as _CloudHsmCreate | ||
| from .aaz.latest.cloudhsm.backup._start import Start as _CloudHsmBackupStart |
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.
does this also need to contain _CloudHsmRestoreStart
|
@necusjz, could you please help review this PR? |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
plz provide the link of your pull request in Azure/aaz. |
…tainerUri and not blobContainerUri
@necusjz PR in AAZ repo: https://github.com/Azure/aaz/pull/790 |

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.