[Storage] az storage account failover: Planned failover GA#32384
[Storage] az storage account failover: Planned failover GA#32384
az storage account failover: Planned failover GA#32384Conversation
️✔️AzureCLI-FullTest
|
|
Hi @calvinhzy, |
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| storage account failover | cmd storage account failover removed property is_preview |
||
| storage account failover | cmd storage account failover update parameter failover_type: added property choices=['Planned', 'Unplanned'] |
|
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. |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Azure CLI storage account failover command to support both planned and unplanned failover types. The changes include updating parameter definitions, removing preview status, expanding test coverage, and improving documentation to distinguish between planned and unplanned failover scenarios.
Key Changes:
- Removed preview status from the
failovercommand - Added
failover_typeparameter with enum validation for 'Planned' and 'Unplanned' values - Expanded test coverage to include planned, unplanned, and default (unplanned) failover scenarios
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test_storage_account_scenarios.py | Added test cases for planned, unplanned, and default failover scenarios with two new storage accounts |
| test_storage_account_failover.yaml | Updated test recording with additional failover test interactions |
| account.py | Reformatted function call for better readability |
| commands.py | Removed is_preview=True flag from failover command |
| _params.py | Added enum type validation and improved help text for failover_type parameter |
| _help.py | Added example for planned failover and clarified unplanned failover usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| while True: | ||
| can_failover = self.cmd('storage account show -n {sa2} -g {rg} --expand geoReplicationStats --query ' | ||
| 'geoReplicationStats.canFailover -o tsv').output.strip('\n') | ||
| if can_failover == 'true': | ||
| break | ||
| time.sleep(10) |
There was a problem hiding this comment.
The polling loop for storage account readiness uses a fixed 10-second interval without a timeout mechanism. This could potentially cause the test to hang indefinitely if the storage account never becomes ready for failover. Consider adding a maximum retry count or timeout to prevent indefinite waiting and provide clearer test failure diagnostics.
| while True: | ||
| can_failover = self.cmd('storage account show -n {sa3} -g {rg} --expand geoReplicationStats --query ' | ||
| 'geoReplicationStats.canFailover -o tsv').output.strip('\n') | ||
| if can_failover == 'true': | ||
| break | ||
| time.sleep(10) |
There was a problem hiding this comment.
This is a duplicate of the same polling pattern without timeout protection. The repeated code suggests this logic should be extracted into a helper method that includes timeout handling and can be reused across all three failover test cases.
Related command
Description
Planned failover GA, add unplanned as a failover type
Testing Guide
History Notes
[Storage]
az storage account failover: AddUnplannedto--failover-typefor Planned failover GAThis 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.