[Storage] az storage account create: Add edge zone support
#17528
Conversation
Juliehzl
requested review from
evelyn-ys,
fengzhou-msft,
houk-ms,
jiasli,
jsntcy and
zhoxing-ms
as code owners
|
Storage |
| @@ -248,6 +248,8 @@ def _type(value): | |||
|
|
|||
| name_type = CLIArgumentType(options_list=['--name', '-n'], help='the primary resource name') | |||
|
|
|||
| edge_zone_type = CLIArgumentType(help='The name of edge zone.', is_preview=True) | |||
There was a problem hiding this comment.
Why call this edge_zone_type? This should be called edge_zone_name imho
There was a problem hiding this comment.
this type is CLI argument type, instead of extended location type.
There was a problem hiding this comment.
Suggestion: add options_list to ensure consistency in all command modules. @Juliehzl
There was a problem hiding this comment.
good point! updated.
| @@ -248,6 +248,8 @@ def _type(value): | |||
|
|
|||
| name_type = CLIArgumentType(options_list=['--name', '-n'], help='the primary resource name') | |||
|
|
|||
| edge_zone_type = CLIArgumentType(help='The name of edge zone.', is_preview=True) | |||
There was a problem hiding this comment.
There are multiple operations in different modules to use the argument, so we define it here. If not in core, each command has to define by themselves.
| @@ -270,6 +270,7 @@ def load_arguments(self, _): # pylint: disable=too-many-locals, too-many-statem | |||
| help='The minimum TLS version to be permitted on requests to storage. ' | |||
| 'The default interpretation is TLS 1.0 for this property') | |||
| c.argument('allow_shared_key_access', allow_shared_key_access_type) | |||
| c.argument('edge_zone', edge_zone_type, min_api='2020-08-01-preview') | |||
There was a problem hiding this comment.
Does it take any string or have a allowed values' list? microsoftrrdclab1 and microsoftlosangeles1 from tests don't seem normal.
There was a problem hiding this comment.
There is no way to get allowed values now unless you know service design. We will add a document link here when service team is ready.
There was a problem hiding this comment.
is_preview is global while min_api is not global. I am confused.
There was a problem hiding this comment.
as different RP has different api version restriction, so we cannot make it global.
There was a problem hiding this comment.
I mean is_preview should not be global.
| @@ -248,6 +248,8 @@ def _type(value): | |||
|
|
|||
| name_type = CLIArgumentType(options_list=['--name', '-n'], help='the primary resource name') | |||
|
|
|||
| edge_zone_type = CLIArgumentType(options_list='--edge-zone', help='The name of edge zone.', is_preview=True) | |||
There was a problem hiding this comment.
How do users know allowed values?
There was a problem hiding this comment.
see #17528 (comment)
Description
To use this feature, we need to register AzureEdgeZone for your subscription.
Wait for 5/4 release with CRP and NRP.
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 feature.
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.