DM-26780: Add --presets for command line bulk overrides via file #377
Conversation
timj
force-pushed
the
tickets/DM-26780
branch
from
55e12e1
to
9d2447c
Compare
| The value of the parameter. | ||
| """ | ||
| ctx.default_map = ctx.default_map or {} | ||
| cmd_name = ctx.info_name |
There was a problem hiding this comment.
Context.info_name is weird: when used with a command that is in a Group, as our subcommands are, then it contains the name of the subcommand, and so this works. But if you were to use this callback with the top level command (butler or pipetask) then it would contain the name of the script, including any extension (.py). Currently this would be ok for our commands because the butler script is called butler (not butler.py), and similar for pipetask. But it might pop up with other scripts if they don't omit the extension. Maybe it's worth stripping .py off the end here if it exists, or warning if .py is found? Or at least it's something to be aware of, someone may hit it someday.
There was a problem hiding this comment.
This effectively becomes the key to use in the dict read from the yaml file. Things are all fine so long as this cmd_name is itself mentioned in the yaml file. Given that we are only using this for subcommands I'm inclined to leave it as is at the moment and let things break.
timj
force-pushed
the
tickets/DM-26780
branch
from
9d2447c
to
8eea089
Compare
| @@ -103,3 +103,12 @@ def makeCollectionType(context, param, value): | |||
| verbose_option = MWOptionDecorator("-v", "--verbose", | |||
| help="Increase verbosity.", | |||
| is_flag=True) | |||
|
|
|||
|
|
|||
| file_override_options = MWOptionDecorator("--options-file", "-@", | |||
There was a problem hiding this comment.
I think the name should be options_file_option; basically _option indicates that it's an option (as opposed to an argument) and options_file_ matches the name of the flag.
timj
force-pushed
the
tickets/DM-26780
branch
from
8eea089
to
7a2c69d
Compare
|
I changed the option name. It was wrong before. |
| @@ -103,3 +103,12 @@ def makeCollectionType(context, param, value): | |||
| verbose_option = MWOptionDecorator("-v", "--verbose", | |||
| help="Increase verbosity.", | |||
| is_flag=True) | |||
|
|
|||
|
|
|||
| options_file_option = MWOptionDecorator("--options-file", "-@", | |||
There was a problem hiding this comment.
of course this adds the -@ short flag for butler as well. I don't mind either way, but I think it will work if you want to add it only in pipetask, where you use the options_file_option you can pass it there:
@click.command()
@options_file_option("-@")
def build(**kwargs):
...
There was a problem hiding this comment.
I want the short flag available everywhere. I realize that butler is not going to be a problem because the options are small but I didn't think it caused harm by having it be consistent.
No description provided.