-
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
{ARM} Bump azure-mgmt-resource from v2019-07-01 to v2020-06-01 #14344
Conversation
add to S173 |
80a33c2
to
ec4016c
Compare
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.
I am OK with vm module
ca579f1
to
ebcd759
Compare
@@ -5973,12 +5984,16 @@ def create_vpn_connection(cmd, resource_group_name, connection_name, vnet_gatewa | |||
deployment_name = 'vpn_connection_deploy_' + random_string(32) | |||
client = get_mgmt_service_client(cmd.cli_ctx, ResourceType.MGMT_RESOURCE_RESOURCES).deployments | |||
properties = DeploymentProperties(template=template, parameters=parameters, mode='incremental') | |||
deployment = Deployment(properties=properties) |
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.
Is it a breaking change?
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.
Yes, it is a breaking change of SDK, and we did made some compatibility logic to ensure that the CLI will not be broken
3f167be
to
aac6ad8
Compare
47b21b8
to
2374d54
Compare
def _deploy_arm_template_at_subscription_scope(cli_ctx, | ||
def _deploy_arm_template_at_subscription_scope(cmd, | ||
template_file=None, template_uri=None, parameters=None, | ||
deployment_name=None, deployment_location=None, validate_only=False, | ||
no_wait=False, no_prompt=False): | ||
deployment_properties = _prepare_deployment_properties_unmodified(cli_ctx=cli_ctx, template_file=template_file, | ||
deployment_properties = _prepare_deployment_properties_unmodified(cli_ctx=cmd.cli_ctx, template_file=template_file, | ||
template_uri=template_uri, parameters=parameters, | ||
mode='Incremental', | ||
no_prompt=no_prompt) | ||
|
||
mgmt_client = _get_deployment_management_client(cli_ctx, plug_pipeline=(template_uri is None)) | ||
mgmt_client = _get_deployment_management_client(cmd.cli_ctx, plug_pipeline=(template_uri is None)) | ||
|
||
validation_result = mgmt_client.validate_at_subscription_scope(deployment_name=deployment_name, properties=deployment_properties, location=deployment_location) | ||
if cmd.supported_api_version(min_api='2019-10-01'): | ||
Deployment = cmd.get_models('Deployment') | ||
deployment = Deployment(properties=deployment_properties, location=deployment_location) | ||
validation_poller = mgmt_client.validate_at_subscription_scope(deployment_name, deployment) | ||
validation_result = LongRunningOperation(cmd.cli_ctx)(validation_poller) | ||
else: | ||
validation_result = mgmt_client.validate_at_subscription_scope(deployment_name, deployment_properties, deployment_location) | ||
|
||
if validation_result and validation_result.error: | ||
err_message = _build_preflight_error_message(validation_result.error) | ||
raise CLIError(err_message) | ||
if validate_only: | ||
return validation_result | ||
|
||
return sdk_no_wait(no_wait, mgmt_client.create_or_update_at_subscription_scope, | ||
deployment_name, deployment_properties, deployment_location) | ||
if cmd.supported_api_version(min_api='2019-10-01'): | ||
return sdk_no_wait(no_wait, mgmt_client.create_or_update_at_subscription_scope, deployment_name, deployment) | ||
return sdk_no_wait(no_wait, mgmt_client.create_or_update_at_subscription_scope, deployment_name, | ||
deployment_properties, deployment_location) |
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.
Compatibility logic for breaking change from SDK
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.
Approved for monitor commands
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.
approve for storage part
d7b4f58
to
515ac0d
Compare
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.
LGTM for Eventhub and Servicebus
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.
For VM module, it's good.
@samkreter Could you please help to review this PR for your part~ |
@@ -1,4 +1,4 @@ | |||
#Wed Jul 22 11:25:50 Pacific Daylight Time 2020 | |||
#Fri Jul 24 17:56:48 China Standard Time 2020 |
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.
Does this comments affect test result? #Closed
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.
This is automatically generated and should not affect the test results #Closed
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.
b5dde9b
to
3f120b8
Compare
3f120b8
to
a043ea6
Compare
/azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
Description
Bump azure-mgmt-resource from v2019-07-01 to v2020-06-01
For PR: #13429 #14241
At the same time, some compatibility logic was made for the breaking change of SDK to ensure that breaking will not be produced on the CLI side, such as: click
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.