Status

StateCompleted
In Release2.0.0
Discussion Thread

[VOTE] AIP-16: CLI: Use nested commands instead of flags

CLI: Use nested commands instead of flags

JIRA

AIRFLOW-3998 - Getting issue details... STATUS

Created

$action.dateFormatter.formatGivenString("yyyy-MM-dd", $content.getCreationDate())

Motivation

The CLI treats `airflow connection` as a single command, with `--list`, `--add`, etc. as flags. This means it's possible to pass options that can't be used together: passing `--list` with `--conn_id` should be invalid. Different flags are also required for listing, adding, and deleting connections. The current implementation has to handle validation of required, invalid, and mutually exclusive options separately for each command. I think the code would be simpler and easier to use if we used nested commands instead of flags: `airflow connections list` and `airflow connections add` would be separate subcommands that would take different arguments, and we wouldn't have to check for invalid combinations of commands and arguments.

Considerations

  • If we're thinking of additional refactoring for the cli module, such as using `click` instead of `argparse`, we might want to combine these changes.
  • It might be possible to make this change without breaking backwards compatibility, but that would make the code a bit more complicated. Should we prefer simpler code or compatibility?

4 Comments

  1. Have you looked at docopt.org? BTW the #1 why not docopt from Click is "very few of these tools deal with nesting of commands and composability in a way like Click" which… yes, docopt has nested commands, and it composes quite differently. But that's, really, the feature.

  2. Daniel Lamblin: I think switching away from argparse is out of scope for this AIP. I wrote a proof of concept patch and didn't have any problems with argparse.


    Ash Berlin-Taylor Tao Feng Xiaodong DENG Fokko Driesprong: what do you think of the proposal? Comments on the discussion thread seem positive, and I'd be happy to flesh out the proof of concept if you're in favor.

    1. Josh Carp  If we could use `argparse` to make this featrue come true. I think we should keep it in out code better, not change to other library temporary.

    2. I'd be happy with sticking with argparse, or with porting the code to use click. It's not a new dep to the project as we're already using it for some of our dev scripts (though on a slightly older version)