-
Notifications
You must be signed in to change notification settings - Fork 111
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Assume positional arguments are required
The 'positional' keyword specifically applies to oslo.config's argparse support. Unlike oslo.config, argparse assumes that all positional arguments are required by default, and you have to explicitly tell it that a positional argument is optional if you'd like to opt into that behavior. This patch adopts that same behavior for oslo.config. When you define an option to be non-positional (oslo.config's default, designed for config files), then oslo.config makes that option optional: However, when you define an option to be positional, oslo.config assumes that the option is primarily going to be used on the CLI and thus sets it as required, by default. This change in behavior has the side effect of allowing argparse to enforce required arguments on the CLI *while* parsing arguments, instead of depending on oslo.config to detect the condition *after* argparse has been allowed to parse "invalid" arguments. argparse correctly raises a SystemExit in this case, and prints the actual command usage and a "hey, you forgot this required argument", instead of allowing oslo.config to dump a backtrace to the CLI with a context-less error message ("context-less" in that no useful CLI usage information is dumped along with the crash to help you correct the condition). Change-Id: Ifdc6918444fe72f7e1649483c237cce64b4c72d8 Partial-Bug: 1676989
- Loading branch information
1 parent
ec84eed
commit 18d1617
Showing
4 changed files
with
34 additions
and
27 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
12 changes: 12 additions & 0 deletions
12
releasenotes/notes/positional-arguments-are-required-22ddca72e6f523bf.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
--- | ||
upgrade: | ||
- | | ||
Positional options are now required by default, to match argparse's default | ||
behavior. To revert this behavior (and maintain optional positional | ||
arguments), you need to explicitly specify ``positional=True, | ||
required=False`` as part of the options definition. | ||
fixes: | ||
- | | ||
On the command line, oslo.config now returns command usage information from | ||
argparse (instead of dumping a backtrace) when required arguments are | ||
missing. |