Specifying --output at top level corrupts subcommand configuration

[nickstenning]

Describe the bug

Using the --output option (and possibly other options) causes subcommands to print incorrect help information, and execute incorrect code paths.

To Reproduce

Use the --output common option before the subcommands. Here are a few examples and the confusing output they generate:

$ az --output=json account
az account: error: the following arguments are required: _subcommand
usage: az account [-h] {list} ...

$ az --output=json account --help

Group
    az account : Manage Azure subscription information.

Commands:
    list : Get a list of subscriptions for the logged in account.

$ az --output=json account list
''
Traceback (most recent call last):
  File "C:\Users\VSSADM~1\AppData\Local\Temp\pip-install-jvxpk9ku\knack\knack\cli.py", line 197, in invoke
  File "C:\Users\VSSADM~1\AppData\Local\Temp\pip-install-jvxpk9ku\azure-cli-core\azure\cli\core\commands\__init__.py", line 307, in execute
KeyError: ''

$ az --output=json account show
az account: 'show' is not in the 'az account' command group. See 'az account --help'.

Expected behavior

Compare the above commands to the results when --output is in the "correct" place:

$ az account --output=json
az account: error: the following arguments are required: _subcommand
usage: az account [-h]
                  {list,set,show,clear,list-locations,get-access-token,lock,management-group}
                  ...

$ az account --help --output=json

Group
    az account : Manage Azure subscription information.

Subgroups:
    lock             : Manage Azure subscription level locks.
    management-group : Manage Azure Management Groups.

Commands:
    clear            : Clear all subscriptions from the CLI's local cache.
    get-access-token : Get a token for utilities to access Azure.
    list             : Get a list of subscriptions for the logged in account.
    list-locations   : List supported regions for the current subscription.
    set              : Set a subscription to be the current active subscription.
    show             : Get the details of a subscription.

$ az account list --output=json
[
    <list of accounts>
]

$ az account show --output=json
{
    <account details>
}

Even if specifying --output at the start of the args list isn't supported, I would expect to see an error or some indication that the argument is unrecognised or invalid at the position given. I do not expect to see an exception (as in the third example) or a failure of the CLI to correctly list subcommands (as in examples one and two).

Environment summary

MSI on Windows 10. Full details below:

`az --version` output

> az --version
azure-cli (2.0.50)
acr (2.1.8)

acs (2.3.11)

advisor (2.0.0)

ams (0.3.0)

appservice (0.2.6)

backup (1.2.1)

batch (3.4.1)

batchai (0.4.4)

billing (0.2.0)

botservice (0.1.1)

cdn (0.2.0)

cloud (2.1.0)

cognitiveservices (0.2.3)

command-modules-nspkg (2.0.2)

configure (2.0.19)

consumption (0.4.0)

container (0.3.8)

core (2.0.50)

cosmosdb (0.2.3)

dla (0.2.3)

dls (0.1.4)

dms (0.1.1)

eventgrid (0.2.0)

eventhubs (0.3.1)

extension (0.2.3)

feedback (2.1.4)

find (0.2.12)

hdinsight (0.1.0)

interactive (0.4.0)

iot (0.3.4)

iotcentral (0.1.3)

keyvault (2.2.6)

lab (0.1.3)

maps (0.3.2)

monitor (0.2.6)

network (2.2.8)

nspkg (3.0.3)

policyinsights (0.1.0)

profile (2.1.2)

rdbms (0.3.4)

redis (0.3.2)

relay (0.1.2)

reservations (0.4.0)

resource (2.1.6)

role (2.1.9)

search (0.1.1)

servicebus (0.3.2)

servicefabric (0.1.7)

signalr (1.0.0)

sql (2.1.5)

storage (2.2.4)

telemetry (1.0.0)

vm (2.2.7)
Python location 'C:\Program Files (x86)\Microsoft SDKs\Azure\CLI2\python.exe'

Extensions directory 'C:\Users\nst.azure\cliextensions'
Python (Windows) 3.6.6 (v3.6.6:4cf1f54eb7, Jun 27 2018, 02:47:15) [MSC v.1900 32 bit (Intel)]
Legal docs and information: aka.ms/AzureCliLegal

[yugangw-msft]

I believe you have to put all commands (main and sub commands) up to front before any arguments following. Not doing that, the error reporting will not be decent

[nickstenning]

Hi @yugangw-msft! Thanks for the quick response. I understand this isn't the intended order, but I think we should give good error messages if the user happens to provide options/subcommands in an unintended order.

This might not be a bug in the sense that providing --output first is supposed to work but doesn't, but it's certainly a user experience bug.

[yonzhan]

add to S164.

[jiasli]

This is actually a very good catch, but given the technical complication and frequency of this usage, I prefer not to fix it.

Workaround

Use the config file to configure global output format.

Analysis

When az --output json {} is run,

1. _rudimentary_get_command -> '':
   

   azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py

   Line 487 in 683b970

   command = self._rudimentary_get_command(args)

2. Trim command table to top layer: vm/network/storage..., each with only one command, as preparation for az and az --help

3. parse_args succeeds if the actual command is in the above list and fails otherwise:
   

   azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py

   Line 554 in 683b970

   parsed_args = self.parser.parse_args(args)

4. If parse_args succeeds, command_table[command] fails due to empty command:
   

   azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py

   Line 566 in 683b970

   command_source = self.commands_loader.command_table[command].command_source

Example:

• az --output json vm list fails at L554 parse_args, because the vm list is not in the trimed command list:

  az vm: 'list' is not in the 'az vm' command group.

• az --output json account list passes L554 parse_args but later fails at L566 command_table[command], because the command is parsed correctly but fails due to empty command:

  KeyError: ''

⚠Changing command at L566 to parsed_args.command won't work either, since global params like --output is added to each parser via parents, so subparsers will override the global --output:

azure-cli/src/azure-cli-core/azure/cli/core/parser.py

Lines 84 to 91 in 2caccaa

	command_parser = subparser.add_parser(command_verb,
	description=metadata.description,
	parents=self.parents,
	conflict_handler='error',
	help_file=metadata.help,
	formatter_class=fc,
	cli_help=self.cli_help,
	_command_source=metadata.command_source)

This is some limitation imposed by argparse and can be verified with a sample:

import argparse

parser = argparse.ArgumentParser(add_help=False)
parser.add_argument('--output', default='json')
subparsers = parser.add_subparsers()

parser_a = subparsers.add_parser('a', parents=[parser])
parser_a.add_argument('--bar')

print(parser.parse_args(['--output', 'yaml', 'a', '--bar', 'Z']))
print(parser.parse_args(['a', '--bar', 'Z', '--output', 'yaml']))

Output:

Namespace(bar='Z', output='json')  # Notice yaml doesn't make it way to the parsed namespace
Namespace(bar='Z', output='yaml')

Also, az help/--help/-h is executed by L554 parse_args.

Another complication is that if _rudimentary_get_command returns '', we can't tell if this is due to no command before '-{}' or invalid command.