{POSTGRESQL} az postgres flexible-server create/upgrade: Update warning for restricted operation
#32699
Conversation
️✔️AzureCLI-FullTest
|
|
Hi @nasc17, |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
️✔️AzureCLI-BreakingChangeTest
|
|
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 error message shown when a location is restricted from performing operations on PostgreSQL flexible servers. The change makes the error message more generic to cover not just provisioning restrictions but other operations like upgrades.
Changes:
- Updated error message from "The location is restricted for provisioning of flexible servers. Please try using another region." to "The location is restricted from performing this operation."
- Added a TODO comment indicating the need for future corrections to capability calls
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if restricted == "Enabled" and not is_offer_restriction_check_required: | ||
| raise InvalidArgumentValueError("The location is restricted for provisioning of flexible servers. Please try using another region.") | ||
| raise InvalidArgumentValueError("The location is restricted from performing this operation.") | ||
|
|
||
| if restricted != "Disabled" and not is_offer_restriction_check_required: | ||
| raise InvalidArgumentValueError("No available SKUs in this location.") |
There was a problem hiding this comment.
These two conditions have overlapping logic. When restricted == "Enabled", the first condition (line 59) will raise an error with the message "The location is restricted from performing this operation." However, if that error is not raised (e.g., due to is_offer_restriction_check_required being True), the second condition (line 62) will also be true since "Enabled" != "Disabled", causing it to raise a different error with the message "No available SKUs in this location."
This creates inconsistent behavior where the same state (restricted == "Enabled") can result in different error messages depending on the value of is_offer_restriction_check_required. Consider using elif for the second condition to prevent this overlap, or adjust the logic to handle the "Enabled" state more explicitly.
| # Update once capability calls are corrected for each command | ||
| if restricted == "Enabled" and not is_offer_restriction_check_required: | ||
| raise InvalidArgumentValueError("The location is restricted for provisioning of flexible servers. Please try using another region.") | ||
| raise InvalidArgumentValueError("The location is restricted from performing this operation.") |
There was a problem hiding this comment.
The error message "The location is restricted from performing this operation." is grammatically awkward because a location cannot perform an operation. Consider rephrasing to something more natural like "This operation is restricted in the selected location." or "The selected location does not support this operation." to better convey that it's the operation that's restricted in that location, not the location performing the operation.
| raise InvalidArgumentValueError("The location is restricted from performing this operation.") | |
| raise InvalidArgumentValueError("This operation is restricted in the selected location.") |
| geo_backup = [feature for feature in supported_features if feature.name == "GeoBackup"] | ||
| autonomous_tuning = [feature for feature in supported_features if feature.name == "IndexTuning"] | ||
|
|
||
| # Update once capability calls are corrected for each command |
There was a problem hiding this comment.
This TODO comment is vague and lacks an actionable plan. It's unclear what "capability calls are corrected for each command" means or what the expected outcome would be. Consider either:
- Making this comment more specific about what needs to be corrected and what the expected behavior should be
- Creating a tracking issue and referencing it in the comment
- Removing the comment if the current behavior is acceptable
Vague TODO comments tend to accumulate technical debt without providing clear guidance for future maintainers.
| # Update once capability calls are corrected for each command |
|
@evelyn-ys @yonzhan please run checks and merge if pass |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
4 participants
Related command
az postgres flexible-server create/upgradeDescription
Was saying provisioning was blocked when could impact other operations such as upgrade
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis 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.