Skip to content

Conversation

@Juliehzl
Copy link
Contributor

@Juliehzl Juliehzl commented Mar 31, 2021

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.

@yonzhan
Copy link
Collaborator

yonzhan commented Mar 31, 2021

Storage

@yonzhan yonzhan added this to the S185 milestone Mar 31, 2021
@Azure Azure deleted a comment from azure-pipelines bot Apr 6, 2021

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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why call this edge_zone_type? This should be called edge_zone_name imho

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this type is CLI argument type, instead of extended location type.

Copy link
Member

@evelyn-ys evelyn-ys Apr 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: add options_list to ensure consistency in all command modules. @Juliehzl

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point! updated.

@yonzhan yonzhan modified the milestones: S185, S186 Apr 8, 2021

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must it be in core?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it take any string or have a allowed values' list? microsoftrrdclab1 and microsoftlosangeles1 from tests don't seem normal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_preview is global while min_api is not global. I am confused.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as different RP has different api version restriction, so we cannot make it global.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean is_preview should not be global.


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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a min_api?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do users know allowed values?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Juliehzl Juliehzl merged commit bcb291a into Azure:dev Apr 16, 2021
@Juliehzl Juliehzl deleted the extended-location branch April 16, 2021 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants