Azure
Fix loader sequence to avoid wrong param info & random change in docs
Related command
Should be all commands when batch export the help files. Examples: az acr query, az ad ds create, az mysql down.
Description
We found random output of arguments while batch getting help files, it is because of the random sequence of command loaders.
The random output is introduced by for looping a set, as set in Python is unordered, every execution can lead to different result, and that makes the help files randomly get wrong parameter detail and wrong parameter group info when exporting it in a batch by calling
load_arguments() in line 487.
After making change to sort the command loader list, the output become steady and correct in my local test. So, we need a steady sequence with the correct order (extension loaders first) here to get a steady and correct output for help files.
For example 1 az ad ds create, when export all help files while installed extension ad, the parameter "subscription" might be loaded by azext_ad.DomainServicesResorceProviderCommandsLoader or azure.cli.command_modules.role.RoleCommandsLoader. If the arguments are first updated by azure.cli.command_modules.role.RoleCommandsLoader, the parameter "subscription" is lacking; If the arguments are first updated by azext_ad.DomainServicesResorceProviderCommandsLoader, the parameter "subscription" will exist and align with the output of az ad ds create -h.
For example 2 az mysql down, when export all help files while installed extension db-up, parameters might be loaded by azext_db_up.DbupCommandsLoader or azure.cli.command_modules.rdbms.RdbmsCommandsLoader. If the arguments are first updated by azure.cli.command_modules.rdbms.RdbmsCommandsLoader, the parameters in help files are:
| Command | Parameter | group_name |
|---|---|---|
| az mysql down | --subscription | Resource Id Arguments |
| az mysql down | --resource-group -g | Resource Id Arguments |
| az mysql down | --server-name -s | Resource Id Arguments |
| az mysql down | --ids | Resource Id Arguments |
If the arguments are first updated by azext_db_up.DbupCommandsLoader, the parameters align with the output of az mysql down -h:
| Command | Parameter | group_name |
|---|---|---|
| az mysql down | --subscription | Global Arguments |
| az mysql down | --resource-group -g | null |
| az mysql down | --server-name -s | null |
For example 3 az acr query, when export all help files while installed extension acrquery, the parameter "username" and "password" might be loaded by azure.cli.command_modules.acr.ACRCommandsLoader or azext_acrquery.AcrqueryCommandsLoader. If the arguments are first updated by azure.cli.command_modules.acr.ACRCommandsLoader, the option lists in the object of these parameters will be ['--username', '-u'] and ['--password', '-p'], which is from azure-cli/src/azure-cli/azure/cli/command_modules/acr/_params.py at main · Azure/azure-cli (github.com), otherwise if the arguments are first updated by azext_acrquery.AcrqueryCommandsLoader, the option lists in the object of these parameters will be ['--username'] and ['--password'], which is from azure-cli-extensions/src/acrquery/azext_acrquery/_params.py at main · Azure/azure-cli-extensions (github.com).
Testing Guide
Check the arguments detail in the help objects of the example commands to make sure no random changes with different runs.
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.
-
[x] The PR title and description has followed the guideline in Submitting Pull Requests.
-
[x] I adhere to the Command Guidelines.
-
[x] I adhere to the Error Handling Guidelines.
Hi @xmdanni, Usually we only allow pull requests to be submitted to the dev branch, please double check your pull request target branch main.
![azure-client-tools-bot-prd[bot] avatar](https://avatars.githubusercontent.com/u/37600188?v=4 "Published by a pub.dev verified publisher"){kind=small}
️✔️AzureCLI-FullTest
️✔️acr
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️acs
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️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
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️cosmosdb
️✔️latest
️✔️3.12
️✔️3.9
️✔️databoxedge
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️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
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️keyvault
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️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
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️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
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️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
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️synapse
️✔️latest
️✔️3.12
️✔️3.9
️✔️telemetry
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️util
️✔️latest
️✔️3.12
️✔️3.9
️✔️vm
️✔️2018-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2019-03-01-hybrid
️✔️3.12
️✔️3.9
️✔️2020-09-01-hybrid
️✔️3.12
️✔️3.9
️✔️latest
️✔️3.12
️✔️3.9
️✔️AzureCLI-BreakingChangeTest
️✔️Non Breaking Changes
Thank you for your contribution! We will review the pull request and get back to you soon.
Please add context in the PR description to help others to understand it's background and purpose~
Please add context in the PR description to help others to understand it's background and purpose~
Updated, thanks. Also the context can be seen in the channel reply
I amended my commit and updated the description. PR is ready for review, thanks! @yonzhan @zhoxing-ms @jsntcy @jiasli
Though I haven't looked deeply into this issue, it doesn't seem to happen to the in-tool help.
To isolate the issue, we first disable command index to make CLI load all modules and extensions:
az config set core.use_command_index=false
Regarding example 1, it is by-design that az ad app create from azure.cli.command_modules.role.RoleCommandsLoader ignores --subscription:
https://github.com/Azure/azure-cli/blob/e8efb791ea2f0ba5e4f172b26f75725fd8aa8079/src/azure-cli/azure/cli/command_modules/role/_params.py#L26
> az ad app create -h
...
Global Arguments
--debug : Increase logging verbosity to show all debug logs.
--help -h : Show this help message and exit.
--only-show-errors : Only show errors, suppressing warnings.
--output -o : Output format. Allowed values: json, jsonc, none, table, tsv,
yaml, yamlc. Default: json.
--query : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--verbose : Increase logging verbosity. Use --debug for full debug logs.
And it is also by design that az ad ds create from azext_ad.DomainServicesResorceProviderCommandsLoader supports --subscription:
> az ad ds create -h
...
Global Arguments
--debug : Increase logging verbosity to show all debug logs.
--help -h : Show this help message and exit.
--only-show-errors : Only show errors, suppressing warnings.
--output -o : Output format. Allowed values: json, jsonc, none, table, tsv,
yaml, yamlc. Default: json.
--query : JMESPath query string. See http://jmespath.org/ for more
information and examples.
--subscription : Name or ID of subscription. You can configure the default
subscription using `az account set -s NAME_OR_ID`.
--verbose : Increase logging verbosity. Use --debug for full debug logs.
Then let's check the source code:
https://github.com/Azure/azure-cli/blob/ccf2bd499dc75e38010346febde63a0b629c34ee/src/azure-cli-core/azure/cli/core/init.py#L492-L502
When az ad ds create -h is run, command will be 'ad ds create'. L499 is hit and only one command loader will be loaded, in this case azext_ad.DomainServicesResourceProviderCommandsLoader:
azure.cli.command_modules.role.RoleCommandsLoader will not be loaded and that's why --subscription is not ignored.
Question: Can the docs generation pipeline correctly handle different presence of --subscription for az ad app create and az ad ds create? In other words, it is incorrect to show --subscription for az ad app create, and it is incorrect to hide --subscription for az ad ds create.
https://github.com/Azure/azure-cli/blob/ccf2bd499dc75e38010346febde63a0b629c34ee/src/azure-cli-core/azure/cli/core/init.py#L492-L502
@jiasli Our docs generation pipeline is calling this api provided by azure-cli to batch export the help files, and the problem is in the output of this azure-cli api. This PR is to fix the issue in azure-cli batch help export api to correctly handle different presence of --subscription and other arguments. In-line help is for calling a single command each time which goes to line 499, it is definitely without this issue. But the batch export does not hit line 499, because to get all help files, the command arg should be None, so it goes to line 493 to 497. The root cause of this problem is in line 494, command_loaders is a set, and set in Python is disordered, every time you iterate a set, the output order is random. This usage introduces the risk in line 502 to directly for loop a disordered set without ordering it, and this disordering leads to random diff in help files. My PR sort the set used by the for loop, to make sure loaders are iterated in a correct order, thus ensure this api return the result both steady and correct.
cc @yonzhan @zhoxing-ms @jsntcy @mtrilbybassett
Whether --subscription is included in the help message is determined when L696 is called:
https://github.com/Azure/azure-cli/blob/7a4d03f077151be8fc312c9ce82febd1dda118cb/src/azure-cli-core/azure/cli/core/init.py#L694-L696
It updates the command's reflection-retrieved arguments based on the information in master_arg_registry.
If azure.cli.command_modules.role.RoleCommandsLoader has been loaded, the ignored _subscription will appear in master_arg_registry, under ad:
When az ad ds create's argument is updated, the ignored _subscription takes effect.
On the other hand, if azure.cli.command_modules.role.RoleCommandsLoader has not been loaded, _subscription won't appear under master_arg_registry. When az ad ds create's argument is updated, _subscription will be kept.
Even though loading the extension first will solve this specific case, this issue still occurs if things reverse: a command module supports _subscription but an extension ignores it. In that case, if the extension is loaded first, the help message for the command module will not have _subscription which is wrong.
For example 1, without changing the current command design such as moving az ad ds to a dedicated command group az ds, the solution is to not ignore _subscription on az ad which includes az ad ds, but on deeper levels - az ad app/sp/user/group.
I haven't looked deeply into example 2 and 3. I will take a deeper look next week.
For example 2, server_name is defined under az mysql with id_part='name':
https://github.com/Azure/azure-cli/blob/70f70f39cdaff028c111a750a3419d5c60590d9c/src/azure-cli/azure/cli/command_modules/rdbms/_params.py#L50-L52
https://github.com/Azure/azure-cli/blob/70f70f39cdaff028c111a750a3419d5c60590d9c/src/azure-cli/azure/cli/command_modules/rdbms/_params.py#L43
The extension db_up alters server_name for az mysql down:
https://github.com/Azure/azure-cli-extensions/blob/4d67f30a9d4542ca71c801de32d577e0b7b03bc6/src/db-up/azext_db_up/_params.py#L42
c.argument('server_name', options_list=['--server-name', '-s'], help='Name of the server.')
For example 3, username and password are defined under az acr with short name -u, -p:
https://github.com/Azure/azure-cli/blob/ccf2bd499dc75e38010346febde63a0b629c34ee/src/azure-cli/azure/cli/command_modules/acr/_params.py#L90-L91
The extension acrquery alters username and password for az acr query:
https://github.com/Azure/azure-cli-extensions/blob/16217261d3ecb66523ee6e833dcfedda0b3d6449/src/acrquery/azext_acrquery/_params.py#L16-L17
c.argument('username', help='Registry username')
c.argument('password', help='Registry password')
Notice: Instead of replacing all properties of an argument as a single object, only specified properties are altered:
https://github.com/microsoft/knack/blob/cd590312b9b4ea1402d7a0a76814f6ca4da7fab8/knack/arguments.py#L41
self.settings.update(**other.settings)
So, in example 2, id_part is preserved for az mysql down; in example 3, options_list is preserved for az acr query.
All these 3 examples have one thing in common: An argument is defined at an upper level (az ad) by a command module and gets altered at a lower level (az ad ds) by an extension.
When a specific command from an extension is run, the core will only load the extension, so that the command module's argument definition won't take effect.
However, in the reference doc generation pipeline, all command modules and extensions are loaded, but in a random order. When a command module is loaded and then a corresponding extension is loaded, the command module's argument definition takes effect, affecting the extension's arguments.
Commenter does not have sufficient privileges for PR 27296 in repo Azure/azure-cli
![azure-pipelines[bot] avatar](https://avatars.githubusercontent.com/in/9426?v=4 "Published by a pub.dev verified publisher"){kind=small}
For a normal command, such as az ad app create -h, command_loaders will be a list containing one element, such as
[<azure.cli.command_modules.role.RoleCommandsLoader object at 0x000002323E65B170>]
For bare az or command group az ad, command is '' or ad, and command_loaders is None, thus no arguments will be loaded at all:
https://github.com/Azure/azure-cli/blob/9f702e6cafacb4b32ac02243d87cc0770f3b5714/src/azure-cli-core/azure/cli/core/init.py#L525
Only when generating reference doc, command can be None, triggering the merging command_loaders logic:
https://github.com/Azure/azure-cli/blob/9f702e6cafacb4b32ac02243d87cc0770f3b5714/src/azure-cli-core/azure/cli/core/init.py#L516-L521
Also see https://github.com/Azure/azure-cli/pull/31029#issuecomment-2720764303
@jiasli @jsntcy @AllyW, I found two more cases in recent random diff, added in this PR description as example 4 & 5.
@xmdanni another way to avoid this random loading issue is iterating through command_table and calling load_arguments by command_name specifically, as azdev did here
Example 4:
In iot module, --hub-name is defined on az iot hub certificate scope:
https://github.com/Azure/azure-cli/blob/cdbbe8e8067c54914236c46d405b0d947d17fcbc/src/azure-cli/azure/cli/command_modules/iot/_params.py#L270-L272
In azure-iot extension, --hub-name/-n is defined on az iot scope:
https://github.com/Azure/azure-iot-cli-extension/blob/395ec8eab42f37b03a97e42c93c4e602dcfd6f5e/azext_iot/_params.py#L102-L106
with self.argument_context("iot") as context:
...
context.argument(
"hub_name", options_list=["--hub-name", "-n"], arg_type=hub_name_type,
help="IoT Hub name. Required if --login is not provided.",
arg_group="IoT Hub Identifier"
)
Example 5:
In appservice modules, --name has id_part, enabling --ids argument:
https://github.com/Azure/azure-cli/blob/0052ebfbeb374246a8dfdb907f93a80fd23cc21d/src/azure-cli/azure/cli/command_modules/appservice/_params.py#L92
https://github.com/Azure/azure-cli/blob/0052ebfbeb374246a8dfdb907f93a80fd23cc21d/src/azure-cli/azure/cli/command_modules/appservice/_params.py#L56-L58
In webapp extension, --name doesn't have id_part on az webapp scan scope:
https://github.com/Azure/azure-cli-extensions/blob/bf4b942f8e269f0ee4ca319c9e6333fc1dc4e37b/src/webapp/azext_webapp/init.py#L38-L39
with self.argument_context('webapp scan') as c:
c.argument('name', options_list=['--name', '-n'], help='Name of the webapp to connect to')