Skip to content
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

{Misc} Remove direct call to msrestazure #29856

Merged
merged 6 commits into from
Oct 8, 2024

Conversation

evelyn-ys
Copy link
Member

@evelyn-ys evelyn-ys commented Sep 6, 2024

Related command

Description

We are planning to drop msrestazure dependency as described in it's disclaimer

This package is deprecated and no longer receives updates

The authentication part of this package has been moved to azure-identity

The other parts of this library are covered by azure-mgmt-core

As such, we will no longer accept PR and fix issues on this project.

So this PR mainly works on migrating

  • functions in msrestazure.tools to azure.mgmt.core.tools
  • msrestazure.azure_exceptions.CloudError to azure.core.exceptions.HttpResponseError

There're some other msrestazure calls left including

We will handle these cases in separate PRs

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.

Copy link

azure-client-tools-bot-prd bot commented Sep 6, 2024

️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️advisor
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️ams
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️apim
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️appconfig
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️appservice
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️aro
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️backup
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️batch
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️batchai
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️billing
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️botservice
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️cdn
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️cloud
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️cognitiveservices
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️compute_recommender
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️config
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️configure
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️consumption
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️container
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️containerapp
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️core
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️dla
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️dls
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️dms
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️eventgrid
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️eventhubs
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️feedback
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️find
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️hdinsight
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️identity
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️iot
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️kusto
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️lab
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️managedservices
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️maps
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️marketplaceordering
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️monitor
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️mysql
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️netappfiles
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️network
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️policyinsights
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️privatedns
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️profile
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️rdbms
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️redis
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️relay
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️resource
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️role
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️search
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️security
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️servicebus
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️serviceconnector
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️servicefabric
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️signalr
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️sql
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️sqlvm
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️storage
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️util
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.11
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.11
️✔️3.12
️✔️3.9

Copy link

azure-client-tools-bot-prd bot commented Sep 6, 2024

️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes

@yonzhan
Copy link
Collaborator

yonzhan commented Sep 6, 2024

Misc

Copy link
Member

@FumingZhang FumingZhang left a comment

Choose a reason for hiding this comment

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

Approve for the change related to acs (AKS, src/azure-cli/azure/cli/command_modules/acs). Thanks!

Copy link
Member

@ambrahma ambrahma left a comment

Choose a reason for hiding this comment

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

RDBMS has reviewed this PR.

@yonzhan
Copy link
Collaborator

yonzhan commented Sep 12, 2024

Please fix conflicting files

@evelyn-ys evelyn-ys requested a review from wanyang7 September 18, 2024 07:07
@jiasli
Copy link
Member

jiasli commented Sep 23, 2024

This is a very big RP. I would suggest creating separate PRs for each change:

  1. Migrate parse_resource_id, resource_id, is_valid_resource_id from msrestazure.tools to azure.mgmt.core.tools
  2. Migrate msrestazure.azure_exceptions.CloudError to azure.core.exceptions.HttpResponseError

@wanyang7
Copy link
Member

Approve for the change related to synapse

@jiasli
Copy link
Member

jiasli commented Sep 25, 2024

I actually did the msrestazure.tools to azure.mgmt.core.tools migration for Azure CLI Core in #20740.

@jiasli
Copy link
Member

jiasli commented Sep 25, 2024

I am actually considering moving parse_resource_id, is_valid_resource_id, resource_id to CLI Core (#29973), so that command modules take no direct dependency on azure.cli.core.arm_tools. If there is a Track 3 SDK in the future, the migration would also be easier.

@evelyn-ys evelyn-ys merged commit 6bbf23a into Azure:dev Oct 8, 2024
61 checks passed
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.