Skip to content

{ARM} Move deployments-owned custom commands into separate file#31432

Open
anthony-c-martin wants to merge 2 commits intoAzure:devfrom
anthony-c-martin:ant/move
Open

{ARM} Move deployments-owned custom commands into separate file#31432
anthony-c-martin wants to merge 2 commits intoAzure:devfrom
anthony-c-martin:ant/move

Conversation

@anthony-c-martin
Copy link
Copy Markdown
Member

Related command
az deployment ...

Description
Non-functional refactor to move deployments-owned custom commands into separate file

Testing Guide
This PR relies on the existing tests.


This checklist is used to make sure that common guidelines for a pull request are followed.

@azure-client-tools-bot-prd
Copy link
Copy Markdown

azure-client-tools-bot-prd bot commented May 8, 2025

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

@azure-client-tools-bot-prd
Copy link
Copy Markdown

Hi @anthony-c-martin,
Since the current milestone time is less than 7 days, this pr will be reviewed in the next milestone.

@azure-client-tools-bot-prd
Copy link
Copy Markdown

azure-client-tools-bot-prd bot commented May 8, 2025

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

@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented May 8, 2025

Thank you for your contribution! We will review the pull request and get back to you soon.

@github-actions
Copy link
Copy Markdown

github-actions bot commented May 8, 2025

The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR.

Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions).
After that please run the following commands to enable git hooks:

pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>

@microsoft-github-policy-service microsoft-github-policy-service bot added the Auto-Assign Auto assign by bot label May 8, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added the ARM az resource/group/lock/tag/deployment/policy/managementapp/account management-group label May 8, 2025
@anthony-c-martin anthony-c-martin force-pushed the ant/move branch 3 times, most recently from ac2308d to f1ac337 Compare May 8, 2025 15:31
@anthony-c-martin
Copy link
Copy Markdown
Member Author

@yonzhan please could you run tests for this?

@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented May 9, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@anthony-c-martin
Copy link
Copy Markdown
Member Author

@zhoxing-ms please could you take a look?

@zhoxing-ms
Copy link
Copy Markdown
Contributor

May I ask what the purpose of this PR is? Is this to avoid too many unrelated modules being affected by the api-version bumping of Resource module?

@anthony-c-martin
Copy link
Copy Markdown
Member Author

May I ask what the purpose of this PR is? Is this to avoid too many unrelated modules being affected by the api-version bumping of Resource module?

Just for code organization - I wanted to centralize all of the commands owned by my team in one place (custom_deployments.py), to avoid a giant file with commands owned by different teams.

In terms of how it's implemented - I'm not a Python expert, so please let me know if there's a better way.

@zhoxing-ms
Copy link
Copy Markdown
Contributor

But I see a lot of duplicate code appearing in custom_deployments.py, and we also need to consider code consistency

@anthony-c-martin
Copy link
Copy Markdown
Member Author

But I see a lot of duplicate code appearing in custom_deployments.py, and we also need to consider code consistency

I've pushed an update to remove the duplication

@anthony-c-martin
Copy link
Copy Markdown
Member Author

@yonzhan please could you re-run tests for this?

@zhoxing-ms assuming checks pass - any objections to merging this soon? It's a pure refactor - I'm not changing any functionality. We have some feature work we'd like to start on in this repo, so it would be ideal if we could complete this before starting the feature work.

@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented May 29, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@anthony-c-martin
Copy link
Copy Markdown
Member Author

@zhoxing-ms assuming checks pass - any objections to merging this soon? It's a pure refactor - I'm not changing any functionality. We have some feature work we'd like to start on in this repo, so it would be ideal if we could complete this before starting the feature work.

@zhoxing-ms - reminder on this

@zhoxing-ms
Copy link
Copy Markdown
Contributor

@anthony-c-martin Could you please re-run some deployment related tests in live mode to check if it meets expectations, because there are big code changes to this PR, we'd better be more cautious

@anthony-c-martin
Copy link
Copy Markdown
Member Author

@anthony-c-martin Could you please re-run some deployment related tests in live mode to check if it meets expectations, because there are big code changes to this PR, we'd better be more cautious

There are a large number of failures even when I run the live tests against the dev branch (using azdev test resource --live), so I'm not able to easily figure out whether this PR has introduced any new failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

act-identity-squad ARM az resource/group/lock/tag/deployment/policy/managementapp/account management-group Auto-Assign Auto assign by bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants