-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[RDBMS] Bugfix: az postgres flexible-server create Remove hardcoded API version from network client. #15392
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
[RDBMS] Bugfix: az postgres flexible-server create Remove hardcoded API version from network client. #15392
Changes from all commits
5c5b143
3550fe2
ffbab04
5fbd423
ecd001e
74e9163
fb7c8b9
05b8af0
ca0b71d
d35ef18
1079b65
4ae1c46
eb56bce
001d306
6e7009e
b52167e
d9a06e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -15,8 +15,7 @@ | |||||
| resource_group_name_type, | ||||||
| get_three_state_flag) | ||||||
| from azure.cli.command_modules.rdbms.validators import configuration_value_validator, validate_subnet, \ | ||||||
| retention_validator, tls_validator, public_access_validator, pg_storage_validator, mysql_storage_validator, tier_validator, \ | ||||||
| pg_sku_name_validator, pg_version_validator, mysql_version_validator, maintenance_window_validator, ip_address_validator | ||||||
| tls_validator, public_access_validator, maintenance_window_validator, ip_address_validator, retention_validator | ||||||
| from azure.cli.core.commands.validators import get_default_location_from_resource_group | ||||||
| from azure.cli.core.local_context import LocalContextAttribute, LocalContextAction | ||||||
|
|
||||||
|
|
@@ -229,24 +228,29 @@ def _flexible_server_params(command_group): | |||||
| with self.argument_context('{} flexible-server create'.format(command_group)) as c: | ||||||
| # Add create mode as a parameter | ||||||
| if command_group == 'postgres': | ||||||
| c.argument('tier', default='GeneralPurpose', options_list=['--tier'], validator=tier_validator, | ||||||
| c.argument('tier', default='GeneralPurpose', options_list=['--tier'], | ||||||
| help='Compute tier of the server. Accepted values: Burstable, GeneralPurpose, Memory Optimized ') | ||||||
| c.argument('sku_name', default='Standard_D2s_v3', options_list=['--sku-name'], validator=pg_sku_name_validator, | ||||||
| c.argument('sku_name', default='Standard_D2s_v3', options_list=['--sku-name'], | ||||||
| help='The name of the compute SKU. Follows the convention Standard_{VM name}. Examples: Standard_D4s_v3 ') | ||||||
| c.argument('storage_mb', default='128', options_list=['--storage-size'], type=int, validator=pg_storage_validator, | ||||||
| c.argument('storage_mb', default='128', options_list=['--storage-size'], type=int, | ||||||
| help='The storage capacity of the server. Minimum is 32 GiB and max is 16 TiB.') | ||||||
| c.argument('version', default='12', options_list=['--version'], validator=pg_version_validator, | ||||||
| c.argument('backup_retention', default=7, type=int, options_list=['--backup-retention'], | ||||||
| help='The number of days a backup is retained. Range of 7 to 35 days. Default is 7 days.', | ||||||
| validator=retention_validator) | ||||||
| c.argument('version', default='12', options_list=['--version'], | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. quick question: how can user know which kind of version is accepted?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Do you mean how they can find out which are the valid versions ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If they pass in a wrong value, they will get a Validation error with the set of acceptable values. |
||||||
| help='Server major version.') | ||||||
| c.argument('zone', options_list=['--zone, -z'], | ||||||
| help='Availability zone into which to provision the resource.') | ||||||
| elif command_group == 'mysql': | ||||||
| c.argument('tier', default='Burstable', | ||||||
| help='Compute tier of the server. Accepted values: Burstable, GeneralPurpose, Memory Optimized ') | ||||||
| c.argument('sku_name', default='Standard_B1ms', options_list=['--sku-name'], | ||||||
| help='The name of the compute SKU. Follows the convention Standard_{VM name}. Examples: Standard_B1ms, Standard_D4s_v3 ') | ||||||
| c.argument('storage_mb', default='10', options_list=['--storage-size'], type=int, validator=mysql_storage_validator, | ||||||
| help='The name of the compute SKU. Follows the convention Standard_{VM name}. Examples: Standard_B1ms, Standard_E16ds_v4 ') | ||||||
| c.argument('storage_mb', default='10', options_list=['--storage-size'], type=int, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
because it is int type. |
||||||
| help='The storage capacity of the server. Minimum is 5 GiB and increases in 1 GiB increments. Max is 16 TiB.') | ||||||
| c.argument('version', default='5.7', options_list=['--version'], validator=mysql_version_validator, | ||||||
| c.argument('backup_retention', default=7, type=int, options_list=['--backup-retention'], | ||||||
| help='The number of days a backup is retained. Range of 7 to 35 days. Default is 7 days.') | ||||||
| c.argument('version', default='5.7', options_list=['--version'], | ||||||
| help='Server major version.') | ||||||
| c.argument('zone', options_list=['--zone, -z'], | ||||||
| help='Availability zone into which to provision the resource.') | ||||||
|
|
@@ -263,9 +267,6 @@ def _flexible_server_params(command_group): | |||||
| c.argument('administrator_login_password', options_list=['--admin-password', '-p'], | ||||||
| help='The password of the administrator. Minimum 8 characters and maximum 128 characters. Password must contain characters from three of the following categories: English uppercase letters, English lowercase letters, numbers, and non-alphanumeric characters.', | ||||||
| arg_group='Authentication') | ||||||
| c.argument('backup_retention', default=7, type=int, options_list=['--backup-retention'], | ||||||
| help='The number of days a backup is retained. Range of 7 to 35 days. Default is 7 days.', | ||||||
| validator=retention_validator) | ||||||
| c.argument('tags', tags_type) | ||||||
| c.argument('public_access', options_list=['--public-access'], | ||||||
| help='Determines the public access. Enter single or range of IP addresses to be included in the allowed list of IPs. IP address ranges must be dash-separated and not contain any spaces. Specifying 0.0.0.0 allows public access from any resources deployed within Azure to access your server. Specifying no IP address sets the server in public access mode but does not create a firewall rule. ', | ||||||
|
|
@@ -290,8 +291,6 @@ def _flexible_server_params(command_group): | |||||
|
|
||||||
| with self.argument_context('{} flexible-server update'.format(command_group)) as c: | ||||||
| c.ignore('assign_identity') | ||||||
| c.argument('backup_retention', type=int, options_list=['--backup-retention'], | ||||||
| help='The number of days a backup is retained. Range of 7 to 35 days. Default is 7 days.', validator=retention_validator) | ||||||
| c.argument('administrator_login_password', options_list=['--admin-password', '-p'], | ||||||
| help='The password of the administrator. Minimum 8 characters and maximum 128 characters. Password must contain characters from three of the following categories: English uppercase letters, English lowercase letters, numbers, and non-alphanumeric characters.',) | ||||||
| c.argument('ha_enabled', options_list=['--high-availability'], arg_type=get_enum_type(['Enabled', 'Disabled']), | ||||||
|
|
@@ -301,12 +300,13 @@ def _flexible_server_params(command_group): | |||||
| c.argument('tags', tags_type) | ||||||
| if command_group == 'mysql': | ||||||
| c.argument('tier', options_list=['--tier'], | ||||||
| help='Compute tier of the server. Accepted values: Burstable, GeneralPurpose, Memory Optimized ') | ||||||
| help='Compute tier of the server. Accepted values: Burstable, GeneralPurpose, Memory Optimized') | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. still prefer getting allowed value from SDK |
||||||
| c.argument('sku_name', options_list=['--sku-name'], | ||||||
| help='The name of the compute SKU. Follows the convention Standard_{VM name}. Examples: Standard_B1ms, Standard_D4s_v3 ') | ||||||
| help='The name of the compute SKU. Follows the convention Standard_{VM name}. Examples: Standard_B1ms, Standard_E16ds_v4 ') | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same |
||||||
| c.argument('storage_mb', options_list=['--storage-size'], type=int, | ||||||
| validator=mysql_storage_validator, | ||||||
| help='The storage capacity of the server. Minimum is 5 GiB and increases in 1 GiB increments. Max is 16 TiB.') | ||||||
| c.argument('backup_retention', type=int, options_list=['--backup-retention'], | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here is to define the retention days but how could I enable/disable delete retention?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It cannot be disabled. By default it is 7 days. But user can pass a value and override the default value. |
||||||
| help='The number of days a backup is retained. Range of 7 to 35 days. Default is 7 days.') | ||||||
| c.argument('auto_grow', arg_type=get_enum_type(['Enabled', 'Disabled']), options_list=['--storage-auto-grow'], | ||||||
| help='Enable or disable autogrow of the storage. Default value is Enabled.') | ||||||
| c.argument('ssl_enforcement', arg_type=get_enum_type(['Enabled', 'Disabled']), | ||||||
|
|
@@ -317,13 +317,14 @@ def _flexible_server_params(command_group): | |||||
| c.argument('replication_role', options_list=['--replication-role'], | ||||||
| help='The replication role of the server.') | ||||||
| elif command_group == 'postgres': | ||||||
| c.argument('tier', options_list=['--tier'], validator=tier_validator, | ||||||
| help='Compute tier of the server. Accepted values: Burstable, GeneralPurpose, Memory Optimized ') | ||||||
| c.argument('sku_name', options_list=['--sku-name'], validator=pg_sku_name_validator, | ||||||
| c.argument('tier', options_list=['--tier'], | ||||||
| help='Compute tier of the server. Accepted values: Burstable, GeneralPurpose, Memory Optimized') | ||||||
| c.argument('sku_name', options_list=['--sku-name'], | ||||||
| help='The name of the compute SKU. Follows the convention Standard_{VM name}. Examples: Standard_D4s_v3 ') | ||||||
| c.argument('storage_mb', options_list=['--storage-size'], type=int, | ||||||
| validator=pg_storage_validator, | ||||||
| help='The storage capacity of the server. Minimum is 32 GiB and max is 16 TiB.') | ||||||
| c.argument('backup_retention', type=int, options_list=['--backup-retention'], | ||||||
| help='The number of days a backup is retained. Range of 7 to 35 days. Default is 7 days.', validator=retention_validator) | ||||||
|
Comment on lines
+320
to
+327
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you have so many duplicate arguments. could you use CLIArgumentType to abstract them and reuse these type. |
||||||
|
|
||||||
| with self.argument_context('{} flexible-server list-skus'.format(command_group)) as c: | ||||||
| c.argument('location', arg_type=get_location_type(self.cli_ctx)) | ||||||
|
|
@@ -412,15 +413,9 @@ def _flexible_server_params(command_group): | |||||
| with self.argument_context('{} flexible-server replica create'.format(command_group)) as c: | ||||||
| c.argument('source_server', options_list=['--source-server'], | ||||||
| help='The name or resource ID of the source server to restore from.') | ||||||
| if command_group == 'mysql': | ||||||
| c.argument('tier', options_list=['--tier'], | ||||||
| help='Compute tier of the server. Accepted values: Burstable, GeneralPurpose, Memory Optimized ') | ||||||
| c.argument('sku_name', options_list=['--sku-name'], | ||||||
| help='The name of the compute SKU. Follows the convention' | ||||||
| ' Standard_{VM name}. Examples: Standard_B1ms, Standard_D4s_v3 ') | ||||||
| if command_group == 'postgres': | ||||||
| c.argument('tier', options_list=['--tier'], validator=tier_validator, | ||||||
| help='Compute tier of the server. Accepted values: Burstable, GeneralPurpose, Memory Optimized ') | ||||||
| c.ignore('location') | ||||||
| c.ignore('sku_name') | ||||||
| c.ignore('tier') | ||||||
|
Comment on lines
+416
to
+418
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The three arguments are totally removed in existing command?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are not using these three args for the command currently |
||||||
|
|
||||||
| with self.argument_context('{} flexible-server replica stop-replication'.format(command_group)) as c: | ||||||
| c.argument('server_name', options_list=['--name', '-s'], help='Name of the server.') | ||||||
|
|
||||||
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.
actually you even don't need to define it here. network module already has the definition and you could just import it.
azure-cli/src/azure-cli/azure/cli/command_modules/network/_client_factory.py
Lines 7 to 10 in 0a90d44