-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Henrydai/implement change state crud0901 #9357
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?
Henrydai/implement change state crud0901 #9357
Conversation
…ngeState' into henrydai/implementChangeStateCRUD0901
…ngeState' into henrydai/implementChangeStateCRUD0901
|
Validation for Breaking Change Starting...
Thanks for your contribution! |
|
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>
|
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 |
|
…ngeState' into henrydai/implementChangeStateCRUD0901
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
This PR introduces a new Azure CLI extension for managing ChangeState resources through the ChangeSafety service. The extension provides CRUD operations (create, update, show, delete) for ChangeState resources at both subscription and resource group levels.
- Adds Azure CLI extension structure with generated AAZ commands and custom implementations
- Implements target parsing with key=value format and supports aliases (e.g.,
rgforresourceGroupName,operationforhttpMethod) - Includes comprehensive unit tests for target parsing and change definition handling
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Package setup configuration for the change-state extension |
| azext_change_state/custom.py | Custom command implementations with target parsing and change definition injection logic |
| azext_change_state/commands.py | Command registration logic |
| azext_change_state/tests/latest/test_change_state.py | Unit tests for custom command functionality |
| azext_change_state/aaz/latest/change_safety/change_state/*.py | Generated AAZ command implementations for CRUD operations |
| azext_change_state/init.py | Extension loader implementation |
| HISTORY.rst | Release history for version 1.0.0b1 |
| README.md | Extension documentation |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
src/change-state/azext_change_state/tests/latest/test_change_state.py
Outdated
Show resolved
Hide resolved
…ngeState' into henrydai/implementChangeStateCRUD0901
…ngeState' into henrydai/implementChangeStateCRUD0901
…ngeState' into henrydai/implementChangeStateCRUD0901
…ngeState' into henrydai/implementChangeStateCRUD0901
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
…ngeState' into henrydai/implementChangeStateCRUD0901
…ngeState' into henrydai/implementChangeStateCRUD0901
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 9357 in repo Azure/azure-cli-extensions |
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
…ngeState' into henrydai/implementChangeStateCRUD0901
…ngeState' into henrydai/implementChangeStateCRUD0901
src/azure-changesafety/README.md
Outdated
| @@ -0,0 +1,49 @@ | |||
| # Azure CLI Change Safety Extension | |||
| Azure CLI extension for managing Change Safety `ChangeState` resources used to coordinate operational changes across Azure targets. | |||
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.
Rename ChangeState to ChangeRecord. Also, please use the text from our API spec md file or run it by PMs as it is not very direct and clear.
src/azure-changesafety/README.md
Outdated
|
|
||
| ## Commands | ||
| ```bash | ||
| az changesafety changerecord create # Create a ChangeState definition for one or more targets. |
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.
please fix renaming of ChangeState to ChangeRecord everywhere
| --links name=Runbook uri=https://contoso.com/runbook | ||
| ``` | ||
|
|
||
| Update the rollout type and add a comment: |
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.
This is not a common scenario or an example we have discussed where the user can update RolloutType. Please use common usage patterns that our existing scenarios require. For example, I would have expected to see the manual touch example as the first one.
|
|
||
| ## Examples | ||
| Create a ChangeState describing a web app rollout: | ||
| ```bash |
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.
AppDeployment example without a stagemap seems odd. Please use examples of the known user scenarios that we understand very well.
|
|
||
| Delete a ChangeState: | ||
| ```bash | ||
| az changesafety changerecord delete -g MyResourceGroup -n changerecord-webapp-rollout --yes |
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.
What if the user does not specify the resourcegroup, what would be the default scope?
| from azext_changesafety._help import helps # pylint: disable=unused-import | ||
|
|
||
|
|
||
| class ChangeStateCommandsLoader(AzCommandsLoader): |
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.
Is this auto generated from swagger? Can we use ChangeRecord in the code?
| long-summary: > | ||
| Provide at least one target definition to describe which resources or operations the ChangeState | ||
| will affect. Targets are expressed as comma or semicolon separated key=value pairs such as | ||
| resourceId=RESOURCE_ID,operation=DELETE. The command is also available through the alias |
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.
Is capitalization necessary?
|
|
||
|
|
||
| @register_command( | ||
| "changesafety changestate delete", |
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.
Is this text user facing? If so, rename changestate to changerecord
…ngeState' into henrydai/implementChangeStateCRUD0901
| ## Examples | ||
| Create a ChangeState describing a web app rollout: | ||
| Create a ChangeRecord for a manual touch operation (e.g., VM maintenance): | ||
| ```bash |
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.
Can we not keep the examples in the cli.md file and here consistent? Also, have you tried these examples to verify that these work? Also, try to keep these examples consistent with the scenarios that we have tested, i.e., DELETE.
Support for managing Microsoft.ChangeSafety/ChangeState, including creating, updating, showing, and deleting changeState resources.
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.