[SQL] az sql mi create/update: Add memory size in gb parameter
#32466
Conversation
UrosKrsticMIPerf
requested review from
calvinhzy and
evelyn-ys
as code owners
|
Validation for Azure CLI Full Test Starting...
Thanks for your contribution! |
|
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>
|
microsoft-github-policy-service
bot
added
Auto-Assign
There was a problem hiding this comment.
Pull request overview
This PR adds a memorySizeInGb parameter to the az sql mi create and az sql mi update commands, leveraging the 2024-08-01-preview API. This enables users to specify the memory size in gigabytes for SQL Managed Instances.
- Added
--memoryparameter to create and update commands for SQL Managed Instances - Implemented parameter handling in custom logic and parameter definitions
- Added comprehensive test coverage and documentation examples
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| test_sql_commands.py | Adds two new test classes to verify memory size parameter functionality in create and update scenarios |
| custom.py | Updates managed_instance_update function to handle memory_size_in_gb parameter and adds logic to set requested_logical_availability_zone to None |
| _params.py | Defines the memory_size_in_gb parameter type and registers it for both create and update commands |
| _help.py | Adds usage examples demonstrating the --memory parameter for create and update commands |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.cmd('sql mi delete --ids {} --yes' | ||
| .format(managed_instance['id']), checks=NoneCheck()) | ||
|
|
||
| class SqlManagedInstanceHermesUpdateScenarioTest(ScenarioTest): |
There was a problem hiding this comment.
Class name 'SqlManagedInstanceHermesUpdateScenarioTest' conflicts with an existing class at line 8682. The new test class should be renamed to reflect its purpose of testing memory size in GB updates, such as 'SqlManagedInstanceMemorySizeInGBUpdateScenarioTest'.
| class SqlManagedInstanceMemorySizeInGBCreateScenarioTest(ScenarioTest): | ||
| @AllowLargeResponse() | ||
| def test_sql_mi_memorysizeingb_create(self): | ||
|
|
||
| subscription_id = '62e48210-5e43-423e-889b-c277f3e08c39' | ||
| group = 'uroskrstic' |
There was a problem hiding this comment.
This test contains hardcoded subscription IDs and resource group names but is missing the @live_only() decorator. Similar tests in the codebase (e.g., lines 8628, 8684) that use hardcoded resources include this decorator. Add @live_only() before the @AllowLargeResponse() decorator to prevent playback test failures.
| class SqlManagedInstanceHermesUpdateScenarioTest(ScenarioTest): | ||
| @AllowLargeResponse() | ||
| def test_sql_mi_memorysizeingb_update(self): | ||
|
|
||
| subscription_id = '62e48210-5e43-423e-889b-c277f3e08c39' | ||
| group = 'uroskrstic' |
There was a problem hiding this comment.
This test contains hardcoded subscription IDs and resource group names but is missing the @live_only() decorator. Similar tests in the codebase (e.g., lines 8628, 8684) that use hardcoded resources include this decorator. Add @live_only() before the @AllowLargeResponse() decorator to prevent playback test failures.
| self.check('sku.family', '{family}'), | ||
| self.check('sku.capacity', '{v_cores}')]) | ||
|
|
||
| # Get the managed instance and check GPv2 flag |
There was a problem hiding this comment.
Comment incorrectly refers to 'GPv2 flag' but the code is actually checking memory size in GB. Update to: '# Get the managed instance and check memory size in gb'.
| # Get the managed instance and check GPv2 flag | |
| # Get the managed instance and check memory size in gb |
| self.cmd('sql mi update -g {rg} -n {managed_instance_name} --gpv2 {is_general_purpose_v2} --memory {memory_size_in_gb}', | ||
| checks=[self.check('memorySizeInGb', '{memory_size_in_gb}')]) | ||
|
|
||
| # Get the managed instance and check memory size in gb |
There was a problem hiding this comment.
[nitpick] Corrected capitalization of 'gb' to 'GB' to match the parameter name 'memorySizeInGb' and maintain consistency with other comments in the codebase.
| # Get the managed instance and check memory size in gb | |
| # Get the managed instance and check memory size in GB |
| options_list=['--memory'], | ||
| arg_type=memory_size_type, | ||
| help='The memory size of the managed instance.' | ||
| 'Memory size must be specified in GB') |
There was a problem hiding this comment.
Missing space between the two help text strings. The help text will display as 'The memory size of the managed instance.Memory size must be specified in GB'. Add a space before the second string: ' Memory size must be specified in GB'.
| 'Memory size must be specified in GB') | |
| ' Memory size must be specified in GB') |
| - name: Create GPv2 managed instance with specified IOPS limit | ||
| text: az sql mi create -g mygroup -n myinstance -l mylocation -i -u myusername -p mypassword --subnet /subscriptions/{SubID}/resourceGroups/{ResourceGroup}/providers/Microsoft.Network/virtualNetworks/{VNETName}/subnets/{SubnetName} -e GeneralPurpose --gpv2 true -f Gen8IH -c 4 --storage 256GB --iops 3000 | ||
| - name: Create managed instance with specified memory size in GB | ||
| text: az sql mi create -g mygroup -n myinstance -l mylocation -i -u myusername -p mypassword --subnet /subscriptions/{SubID}/resourceGroups/{ResourceGroup}/providers/Microsoft.Network/virtualNetworks/{VNETName}/subnets/{SubnetName} -e GeneralPurpose --gpv2 true -f Gen8IM -c 4 --storage 256GB --iops 3000 --memory 40GB |
There was a problem hiding this comment.
The example uses '40GB' without a space, but CLI parameter values typically don't include units. The value should be '40' (the number only), as the help text in _params.py specifies 'Memory size must be specified in GB'. Update to: --memory 40.
| - name: Update managed instance to GPv2 with specified IOPS limit | ||
| text: az sql mi update -g mygroup -n myinstance -e GeneralPurpose --gpv2 true --iops 3000 | ||
| - name: Update managed instance to use a specified memory size in GB | ||
| text: az sql mi update -g mygroup -n myinstance -e GeneralPurpose --memory 40GB |
There was a problem hiding this comment.
The example uses '40GB' without a space, but CLI parameter values typically don't include units. The value should be '40' (the number only), as the help text in _params.py specifies 'Memory size must be specified in GB'. Update to: --memory 40.
| text: az sql mi update -g mygroup -n myinstance -e GeneralPurpose --memory 40GB | |
| text: az sql mi update -g mygroup -n myinstance -e GeneralPurpose --memory 40 |
3 participants
Related command
az sql mi create
az sql mi update
Description
Added memorySizeInGb parameter from the 2024-08-01-preview API to the az cli client. az sql mi create and update commands are affected.
Testing Guide
Added tests that test this new feature. Example of update command:
az sql mi update -g uroskrstic -n uroskrstic-flexi-test-azpowershell --memory 80 --gpv2 True
This 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.