Unable to pass space-separated values with leading dashes

[jiasli]

Requirement

To run az synapse spark job submit, the user wants to pass --instance, trainer as values to --arguments.

Expected JSON:

"arguments": [
  "--instance",
  "trainer"
]

Symptom

--arguments is defined as

azure-cli/src/azure-cli/azure/cli/command_modules/synapse/_params.py

Line 383 in 39c9174

c.argument('command_line_arguments', options_list=['--arguments'], nargs='+',

So the user tries to pass --instance, trainer separated by spaces, and the command fails

# Line breaks for legibility
az synapse spark job submit --name foo --workspace-name personalizer-trainer --spark-pool-name persbackend --main-definition-file foo --main-class-name org.personalizer.backend.PipelineManager --executors 4 --executor-size Medium 
                            --arguments --instance trainer
argument --arguments: expected at least one argument

As discussed in #16044 (comment), it is possible to use an equal sign = to concatenate the argument name and value for single-value argument:

az keyvault secret set -n mysecret --vault-name mykeyvault --value=-secret

This usage is not documented by argparse: https://docs.python.org/3/library/argparse.html, https://docs.python.org/3/howto/argparse.html

However, it doesn't work for multi-value arguments:

# Line breaks for legibility
az synapse spark job submit --name foo --workspace-name personalizer-trainer --spark-pool-name persbackend --main-definition-file foo --main-class-name org.personalizer.backend.PipelineManager --executors 4 --executor-size Medium 
                            --arguments=--instance trainer
unrecognized arguments: trainer

Workarounds

There are several workaround worth considering:

1. Instead of using a nargs='+', --arguments can take single value and split it with shlex.split

2. Use nargs=argparse.REMAINDER to make --arguments take all remaining arguments. azdev uses this approach for --pytest-args and perhaps this is the best way to go.

   However, this usage has been removed from argparse document since Python 3.9 (https://bugs.python.org/issue17050):

   Since this feature is buggy, and there isn't an easy fix, we should probably remove any mention of it from the docs. We can still leave it as an undocumented legacy feature.

3. Replace --arguments with the -- convention of argparse to mark all remaining arguments as positional arguments (https://docs.python.org/3/library/argparse.html#arguments-containing)

4. Add some special handling in --arguments so that dashes can be escaped, like __instance, or ^--instance. But using characters like `, \ may cause other trouble with shell's interpretation.

No matter which workaround we choose, it is not an easy fix.

[ghost]

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @aim-for-better, @idear1203.

Issue Details

---

Requirement

To run az synapse spark job submit, the user wants to pass --instance, trainer as values to --arguments.

Expected JSON:

"arguments": [
  "--instance",
  "trainer"
]

Symptom

--arguments is defined as

azure-cli/src/azure-cli/azure/cli/command_modules/synapse/_params.py

Line 383 in 39c9174

c.argument('command_line_arguments', options_list=['--arguments'], nargs='+',

So the user tries to pass --instance, trainer separated by spaces, and the command fails

# Line breaks for legibility
az synapse spark job submit --name foo --workspace-name personalizer-trainer --spark-pool-name persbackend --main-definition-file foo --main-class-name org.personalizer.backend.PipelineManager --executors 4 --executor-size Medium 
                            --arguments --instance trainer
argument --arguments: expected at least one argument

As discussed in #16044 (comment), it is possible to use an equal sign = to concatenate the argument name and value for single-value argument:

az keyvault secret set -n mysecret --vault-name mykeyvault --value=-secret

This usage is not documented by argparse: https://docs.python.org/3/library/argparse.html, https://docs.python.org/3/howto/argparse.html

However, it doesn't work for multi-value arguments:

# Line breaks for legibility
az synapse spark job submit --name foo --workspace-name personalizer-trainer --spark-pool-name persbackend --main-definition-file foo --main-class-name org.personalizer.backend.PipelineManager --executors 4 --executor-size Medium 
                            --arguments=--instance trainer
unrecognized arguments: trainer

Workarounds

There are several workaround worth considering:

1. Instead of using a nargs='+', --arguments can take single value and split it with shlex.split

2. Use nargs=argparse.REMAINDER to make --arguments take all remaining arguments. azdev uses this approach for --pytest-args and perhaps this is the best way to go.

   However, this usage has been removed from argparse document since Python 3.9 (https://bugs.python.org/issue17050):

   Since this feature is buggy, and there isn't an easy fix, we should probably remove any mention of it from the docs. We can still leave it as an undocumented legacy feature.

3. Replace --arguments with the -- convention of argparse to mark all remaining arguments as positional arguments (https://docs.python.org/3/library/argparse.html#arguments-containing)

4. Add some special handling in --arguments so that dashes can be escaped, like __instance, or ^--instance. But using characters like `, \ may cause other trouble with shell's interpretation.

No matter which workaround we choose, it is not an easy fix.

Author:	jiasli
Assignees:	jiasli
Labels:	Service Attention, Synapse, argparse
Milestone:	Backlog

[jiasli]

Below is why argparse doesn't support multi-value arguments when = is used.

In the --password=-secret solution, the value -secret is called an explicit argument (explicit_arg):

https://github.com/python/cpython/blob/db3ff76da19004f266b62e98a81bdfd322861436/Lib/argparse.py#L2194-L2199

        # if the option string before the "=" is present, return the action
        if '=' in arg_string:
            option_string, explicit_arg = arg_string.split('=', 1)
            if option_string in self._option_string_actions:
                action = self._option_string_actions[option_string]
                return action, option_string, explicit_arg

Then argparse checks if the action represented by option_string takes this one explicit_arg:

https://github.com/python/cpython/blob/db3ff76da19004f266b62e98a81bdfd322861436/Lib/argparse.py#L1948-L1951

                # if there is an explicit argument, try to match the
                # optional's string arguments to only this
                if explicit_arg is not None:
                    arg_count = match_argument(action, 'A')

If so, store the value to the argument represented by option_string:

https://github.com/python/cpython/blob/db3ff76da19004f266b62e98a81bdfd322861436/Lib/argparse.py#L1970-L1976

                    # if the action expect exactly one argument, we've
                    # successfully matched the option; exit the loop
                    elif arg_count == 1:
                        stop = start_index + 1
                        args = [explicit_arg]
                        action_tuples.append((action, args, option_string))
                        break

Notice the line args = [explicit_arg] which indicates only one element is accepted (i.e. explicit_arg) when explicit argument is used.

[jiasli]

Copying my comments from #20663 (comment).

For solution 1 (the shlex.split approach), including quotes in --extra-options's value can become very complex in different shells. For some examples, see #20702 (comment), #20702 (comment).

Now I begin to more and more root for solution 3 (the -- approach):

az storage copy -s source_file -d container_url -- --bla="bla" --foo="foo" --bar="bar"

This is a common convention in Linux world:

• https://unix.stackexchange.com/questions/11376/what-does-double-dash-mean
• https://stackoverflow.com/questions/13321458/meaning-of-git-checkout-double-dashes

[jiasli]

Conclusion

We decided to use option 3 as the solution for argument passthrough.

3. Replace --arguments with the -- convention of argparse to mark all remaining arguments as positional arguments (https://docs.python.org/3/library/argparse.html#arguments-containing)

Positional argument extra_options in az storage copy (#20702) and positional argument ssh_args in ssh vm (Azure/azure-cli-extensions#3698) now follow this convention:

azure-cli/src/azure-cli/azure/cli/command_modules/storage/_params.py

Lines 1292 to 1294 in 215a8b9

	c.positional('extra_options', nargs='*', is_experimental=True, default=[],
	help="Other options which will be passed through to azcopy as it is. "
	"Please put all the extra options after a `--`")

> az storage copy --help
...
Positional
    <EXTRA_OPTIONS>          [Experimental] : Other options which will be passed through to
                                              azcopy as it is. Please put all the extra options
                                              after a `--`.
...
    Upload a single file to Azure Blob using url with azcopy options pass-through.
        az storage copy -s /path/to/file.txt -d
        https://[account].blob.core.windows.net/[container]/[path/to/blob] -- --block-size-mb=0.25
        --check-length

This is a common convention in Linux world:

• https://unix.stackexchange.com/questions/11376/what-does-double-dash-mean
• https://stackoverflow.com/questions/13321458/meaning-of-git-checkout-double-dashes

We also got feedback that this is an elegant solution (#20663 (comment)).