-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[Container Registry] Migrate to track2 SDK #18611
Changes from all commits
6186c41
c4f6ebf
a4d85a0
f359fca
89a5210
b8deb83
f5e7d6c
6942bec
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 |
---|---|---|
|
@@ -9,9 +9,9 @@ | |
from knack.util import CLIError | ||
from knack.log import get_logger | ||
|
||
from msrestazure.azure_exceptions import CloudError | ||
from azure.cli.core.commands import LongRunningOperation | ||
from azure.cli.core.commands.parameters import get_resources_in_subscription | ||
from azure.core.exceptions import ResourceNotFoundError | ||
|
||
from ._constants import ( | ||
REGISTRY_RESOURCE_TYPE, | ||
|
@@ -198,7 +198,8 @@ def get_validate_platform(cmd, platform): | |
"""Gets and validates the Platform from both flags | ||
:param str platform: The name of Platform passed by user in --platform flag | ||
""" | ||
OS, Architecture = cmd.get_models('OS', 'Architecture') | ||
OS, Architecture = cmd.get_models('OS', 'Architecture', operation_group='runs') | ||
|
||
# Defaults | ||
platform_os = OS.linux.value | ||
platform_arch = Architecture.amd64.value | ||
|
@@ -285,10 +286,14 @@ def get_custom_registry_credentials(cmd, | |
:param str password: The password for custom registry (plain text or a key vault secret URI) | ||
:param str identity: The task managed identity used for the credential | ||
""" | ||
Credentials, SourceRegistryCredentials, CustomRegistryCredentials, SecretObject, \ | ||
SecretObjectType = cmd.get_models( | ||
'Credentials', 'CustomRegistryCredentials', 'SourceRegistryCredentials', 'SecretObject', | ||
'SecretObjectType', | ||
operation_group='tasks') | ||
jsntcy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
source_registry_credentials = None | ||
if auth_mode: | ||
SourceRegistryCredentials = cmd.get_models('SourceRegistryCredentials') | ||
source_registry_credentials = SourceRegistryCredentials( | ||
login_mode=auth_mode) | ||
|
||
|
@@ -301,11 +306,6 @@ def get_custom_registry_credentials(cmd, | |
if not username and not password: | ||
is_identity_credential = identity is not None | ||
|
||
CustomRegistryCredentials, SecretObject, SecretObjectType = cmd.get_models( | ||
'CustomRegistryCredentials', | ||
'SecretObject', | ||
'SecretObjectType') | ||
|
||
if not is_remove: | ||
if is_identity_credential: | ||
custom_reg_credential = CustomRegistryCredentials( | ||
|
@@ -328,17 +328,15 @@ def get_custom_registry_credentials(cmd, | |
|
||
custom_registries = {login_server: custom_reg_credential} | ||
|
||
Credentials = cmd.get_models('Credentials') | ||
return Credentials( | ||
source_registry=source_registry_credentials, | ||
custom_registries=custom_registries | ||
) | ||
|
||
|
||
def build_timers_info(cmd, schedules): | ||
TimerTrigger, TriggerStatus = cmd.get_models( | ||
'TimerTrigger', 'TriggerStatus') | ||
timer_triggers = [] | ||
TriggerStatus, TimerTrigger = cmd.get_models('TriggerStatus', 'TimerTrigger', operation_group='tasks') | ||
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. when operation_group is empty it will try to load from the general sdk right? 2020 in this case 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. When operation_group is empty it will load the default version |
||
|
||
# Provide a default name for the timer if no name was provided. | ||
for index, schedule in enumerate(schedules, start=1): | ||
|
@@ -507,11 +505,14 @@ def create_default_scope_map(cmd, | |
raise CLIError('The default scope map was already configured with different repository permissions.' + | ||
'\nPlease use "az acr scope-map update -r {} -n {} --add <REPO> --remove <REPO>" to update.' | ||
.format(registry_name, scope_map_name)) | ||
except CloudError: | ||
except ResourceNotFoundError: | ||
pass | ||
logger.info('Creating a scope map "%s" for provided permissions.', scope_map_name) | ||
poller = scope_map_client.create(resource_group_name, registry_name, scope_map_name, | ||
actions, scope_map_description) | ||
scope_map_request = { | ||
'actions': actions, | ||
'scope_map_description': scope_map_description | ||
} | ||
poller = scope_map_client.begin_create(resource_group_name, registry_name, scope_map_name, scope_map_request) | ||
jsntcy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
scope_map = LongRunningOperation(cmd.cli_ctx)(poller) | ||
return scope_map | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ def acr_agentpool_create(cmd, | |
registry, resource_group_name = get_registry_by_name( | ||
cmd.cli_ctx, registry_name, resource_group_name) | ||
|
||
AgentPool = cmd.get_models('AgentPool') | ||
AgentPool = cmd.get_models('AgentPool', operation_group='agent_pools') | ||
|
||
agentpool_create_parameters = AgentPool( | ||
location=registry.location, | ||
|
@@ -43,10 +43,10 @@ def acr_agentpool_create(cmd, | |
) | ||
|
||
try: | ||
return client.create(resource_group_name=resource_group_name, | ||
registry_name=registry_name, | ||
agent_pool_name=agent_pool_name, | ||
agent_pool=agentpool_create_parameters) | ||
return client.begin_create(resource_group_name=resource_group_name, | ||
registry_name=registry_name, | ||
agent_pool_name=agent_pool_name, | ||
agent_pool=agentpool_create_parameters) | ||
jsntcy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
except ValidationError as e: | ||
raise CLIError(e) | ||
|
||
|
@@ -61,11 +61,15 @@ def acr_agentpool_update(cmd, | |
_, resource_group_name = validate_managed_registry( | ||
cmd, registry_name, resource_group_name) | ||
|
||
AgentPoolUpdateParameters = cmd.get_models('AgentPoolUpdateParameters', operation_group='agent_pools') | ||
|
||
update_parameters = AgentPoolUpdateParameters(count=count) | ||
|
||
Comment on lines
+64
to
+67
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. Are these changes related to how the SDK is being loaded now? 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. Unified operation parameter style We have two ways to load these parameters, model style and json style. example |
||
try: | ||
return client.update(resource_group_name=resource_group_name, | ||
registry_name=registry_name, | ||
agent_pool_name=agent_pool_name, | ||
count=count) | ||
return client.begin_update(resource_group_name=resource_group_name, | ||
jsntcy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
registry_name=registry_name, | ||
agent_pool_name=agent_pool_name, | ||
update_parameters=update_parameters) | ||
except ValidationError as e: | ||
raise CLIError(e) | ||
|
||
|
@@ -84,9 +88,9 @@ def acr_agentpool_delete(cmd, | |
user_confirmation("Are you sure you want to delete the agentpool '{}' in registry '{}'?".format( | ||
agent_pool_name, registry_name), yes) | ||
try: | ||
response = client.delete(resource_group_name=resource_group_name, | ||
registry_name=registry_name, | ||
agent_pool_name=agent_pool_name) | ||
response = client.begin_delete(resource_group_name=resource_group_name, | ||
jsntcy marked this conversation as resolved.
Show resolved
Hide resolved
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 result is not polled. Then at L104,
This is similar to #17185. |
||
registry_name=registry_name, | ||
agent_pool_name=agent_pool_name) | ||
|
||
if no_wait: | ||
logger.warning("Started to delete the agent pool '%s': %s", agent_pool_name, response.status()) | ||
|
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.
why this operation group as default?
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.
We just want
2020-11-01-preview
as default, the webhooks is belong to2020-11-01-preview
.You can refer to other modules that use multiple versions. like monitor
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.
oh so by choosing webhooks, because it isn't in one of the other profiles like agent_pools? If so can we use a more general operation_group name like
registries
? Also shouldn't this work fine without specifying operation_group, given the loader would use the default operation_group which should use default api version?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.
In this profile, we defined a default apiversion and some other groups using special version.
But in the base class ACRCommandsLoader, it doesn't know which one should be used as default.
Why we use
webhooks
? Actually we use other group is ok. Because it will use default apiversion2020-11-01-preview
when group name is not in special group list defined in profile.I just want to tell ACRCommandsLoader to use
2020-11-01-preview
as default version.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.
What happens if we don't set
operation_group='webhooks'
? What is the default version in this case?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.
When we run
get_models
, we need a parameteroperation_group
to tellget_models
to use which model in which api version, like this line.If we don't clarify using which api_version/operation_group, then it will find it from AzLoader
This is why we need this
operation_group
here.In monitor, it rarely uses
get_models
and always clarifyoperation_group
when using. This's why you don't findoperation_group
in AzLoader of monitor.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.
So when
operation_group='webhooks'
is not set,get_models
can't get the default api-version ofResourceType.MGMT_CONTAINERREGISTRY
, right?