pyinvoke / invoke

Pythonic task management & command execution.
http://pyinvoke.org
BSD 2-Clause "Simplified" License
4.4k stars 367 forks source link

*args and **kwds support in tasks #378

Open pandey1010 opened 8 years ago

pandey1010 commented 8 years ago

Is it possible to have support for args and/or kwds in tasks? I need it sometimes to pass all argument commands, as is, to another script.

bitprophet commented 8 years ago

I feel like there's an open ticket for this already, but I can't find it. Semi-related issues that are not duplicates: #174, #9, #159.


Is it possible? Not at the moment, since it makes multi-task CLI parsing quite ambiguous; there's no obvious "one right way" to implement it. I.e.:

$ inv mytask a bunch of things I want to go into splat args here

You probably want that to act like:

mytask(*['a', 'bunch', 'of', 'etc'])

But what if your task collection has another task called bunch? It's then impossible to tell the above apart from an intent to call mytask(*['a']) followed by bunch('of', 'things', 'etc').


Hopefully there's a way to square these two concerns; offhand:

SwampFalc commented 7 years ago

Since I could usefully use an *args for a project I'm working on, I'd like to add my vote for this feature.

If this feature were to come at the cost of being able to have multiple tasks in one invocation, then I personally wouldn't mind. There are many ways in which you can chain tasks, but there is no other way to have an arbitrary amount of arguments for a task.

Otherwise, one other option is that, if the parser knows about splat-args, then it will accept multiple instances of the same argument.

Ie. you would call this as invoke mytesk -a just -a a -a bunch -a of -a arguments, but only if you used *args.

In my own use case, f.ex. I need to specify multiple recipients. I know in advance which argument I might need to repeat, which I do assume is the most likely use case anyway...

bitprophet commented 7 years ago

Thanks for the input, @SwampFalc. (FYI - the inv mytask -a foo -a bar -a biz -a baz concept is arguably covered under #132 - though how that intersects with a splat-args signature may need some thought.)

iago1460 commented 7 years ago

It would be great to have this feature, I would like to use invoke as a entrypoint to pass arguments to other commands

elsimir commented 7 years ago

Hi, Just chiming in to give my support for this feature.

I was just wondering what you think about throwing an exception in the case of ambiguity, but allowing the user to wrap an argument in "quotes" if it has the same name as a task and using that or something similar as a marker that it should always be treated as an argument not a task name.

bitprophet commented 7 years ago

Noting that the word 'kwargs' does not appear in here so searching for the term doesn't find this ticket. Kind of important. (This bump courtesy of a soon to be closed duplicate :))

Yes, having this is still something I'd like. Also, I note the discussion so far has been about just args/varargs, and not kwargs/keyword arguments. Not sure how we'd handle actual kwargs...brainstorm:

dblandin commented 6 years ago

We have a bunch of tasks that rely on **kwargs. Porting over some ability to pass in arbitrary key/value pairs would be fantastic. Agreed that the foo=bar style might not follow the new design direction. Converting any unknown key/value flags to **kwargs sounds pretty good to me.

Here are a few concrete examples in our task collection:

@task
def scale(*args, **kwargs):
    """
    Update desired count for services.

    Usage:

    fab ecs.scale:app-nginx=4,api-nginx=4
    """
@task
def set(pattern, **kwargs):
    """
    Set ENV vars for a service.

    Usage:

    fab ecs.config.set:"app-.*",FOO=bar,BAZ=nie
    """
@task
def put(service_name, **kwargs):
    """
    Set parameters in SSM Parameter Store.

    Usage:

    fab ssm.params.put:<service>,[key_filter=value],[key_filter2=value],[type=SecureString],...
    fab ssm.params.put:prod_app,one=1
    fab ssm.params.put:prod_app,one=1,two=2,type=String
    """
@task
def rollback(env="prod", **kwargs):
    """
    Rollback ecs images / lambda functions to specified or previous versions
    and trigger an ops ci build.

    Usage:

     fab app.rollback:prod,app_image_ref=codeclimate/example:b100
    """
haydenflinner commented 6 years ago

Got it working! In python 3 at least. Won't be able to test or tinker for a bit, let me know if y'all have any ideas or fixes or issues!

print(args, kwargs) - ('a', 'b', 'c', 'd', 'e', 'f', 'g') {'kwargsmadeit': 'x'}
------------------------------------------------------------
~/code/invokestuf(c*) » invoke -d myfuncname a b c d --kwargsmadeit x e f  

~PR coming within about ten minutes, gotta add comments explaining limitations and next steps (signature must be ctx, *args, **ctx)~


    # For least surprise, I think it's important that *args take precedence
    # over optional parameters.

Btw thank you @bitprophet and friends for such clean, easy to read and extend code! I was very pleasantly surprised with how easy it was to at least get kwargs working. Varargs took about 2x the time :smile:

haydenflinner commented 6 years ago

Realized I forgot to note -- This patch is about half the size and appears to work flawlessly with no function signature compromises if you only do *kwargs support. args caused the function signature compromise, because I can't figure out how to place *args where they belong instead of in positional_params[1:], which causes two values to be supplied for required params (because required params are in the kwargs dict that gets unpacked in __call__. Knowing more about the function signature is probably the fix. Probably trivial for someone more experienced with the code or python metaprogramming.

haydenflinner commented 6 years ago

@dblandin Try pip install magicinvoke and let me know if you have any issues. I'm basically using it as a beta branch for the PR I write for invoke, one of the things I added is kwargs

https://magicinvoke.readthedocs.io/en/latest/examples/args-kwargs/README.html#args-kwargs

dblandin commented 6 years ago

Hey @haydenflinner, I no longer have access to the same fabric/invoke-based command suite so I might not be able to test this without spinning up something new. I'm going to mention a former colleague who might be interested.

cc/ @wfleming

johnraz commented 5 years ago

I would be interested in being able to optin for this behavior too, @bitprophet / @haydenflinner what can be done to make this move further?

haydenflinner commented 5 years ago

@johnraz You could try out my patch by doing pip install magicinvoke (leave your tasks the same, just add one with kwargs; it should override the installed invoke, but if not, pip uninstall invoke; its been working fine for me at work so I put it on PyPi in case anyone needs it. Not sure I'll have any time to do merging work here.

johnraz commented 5 years ago

Rethinking this after a night of sleep I realise that my need is slightly different.

What I'd like to achieve is the following:

invoke my-cmd-wrapper pos-arg-1 pos-arg-2 --option1 value

and in my tasks

@task
def my_cmd_wrapper(c, cmd_params):
    c.run("my-cmd {}".format(cmd_params))

Where cmd_params == "pos-arg-1 pos-arg-2 --option1 value"

I know I can achieve this by wrapping everything but the command name within quotes, but getting rid of the quotes is exactly what I'd like to do.

I'm not really sure how this could be achieved by the parser nor if it fits in this issue. I can open a new one if you feel it doesn't fit.

Also please let me know if you find my use case can be better solved another way 😉

haydenflinner commented 5 years ago

You might try only using *args, not **kwargs; perhaps it will parse the 'flags' as values and you can pass *args into the next command directly. Haven't tried.

neighthan commented 5 years ago

@johnraz I have the same use case as you (and which I think also aligns with OP's desire, though they were hoping to achieve this through *args/**kwargs) of wanting to wrap another script with an invoke task and be able to pass the commands through. I just made a few changes in a fork that allow this (see here; probably <10 lines changed). Basically, each task has a flag allow_unknown for whether to allow unknown arguments to be entered for it. This is passed from the task to its parser context and then used when parsing happens to determine behavior on unknown arguments. Then inside of the task you can just access sys.argv to do whatever you want with the passed arguments. Here's an example of how this can be used:

@invoke.task(allow_unknown=True)
def test(ctx) -> None:
    pytest_args = sys.argv[sys.argv.index("test") + 1:]
    cmd = "poetry run pytest " + " ".join(pytest_args)
    ctx.run(cmd)

Before my changes, trying to add coverage to pytest would give the following:

$ invoke test --cov=.
No idea what '--cov' is!

After, it works =) You could still call, e.g. invoke clean test --cov=., but no other tasks should be passed after a task that has allow_unknown = True. If there's interest I can submit a PR for this, but it seems like PRs aren't really being merged right now.

nyurik commented 5 years ago

I have been playing with magicinvoke that has *args, **kwargs support, and I found the automatic parsing of flags into **kwargs to be more of a nuisance than help. My use cases were actually "pass-through", i.e.

def kubectl(c, *args, **kwargs):
    """Run kubectl command with any parameters"""
    c.run(f"{kubectl_cmd()} {escape(*args, **kwargs)}")

def ssh(c, *args, **kwargs):
    """Find Kubernetes pod by name (first param) and ssh to it with any parameters"""
    c.run(f"{kubectl_cmd()}  {find_pod(args[1])}  {escape(*args[1:], **kwargs)}")

In the above examples, I have to re-combine kwargs back into a --key1 val1 -k2 val2 string to pass it to the underlying command, i.e. kubectl or ssh. This is buggy because some programs don't follow the convention of one dash is for one letter flags, and two dashes - for longer flags. Plus some commands may not support = separator.

I would recommend the following strategy:

  1. if task def has no * and ** (current behavior), expect N positional arguments, plus any dash-params, after which the very next positional argument is the name of the next task. The N is 0 or more, depending on:
    • number of task parameters without default values
    • plus any parameters with defaults, but listed in the task(positional=[...])
  2. if task def has a * but no **, treat the rest of the CLI arguments as belonging to this task, first resolve all the positional args (as in # 1), and put the rest of arguments unparsed into the *args array
  3. if task def has * and **, parse any --param val, -p val, etc into the **kwargs, and positional into *args. Note that this (from my perspective) is the least useful option, so it could be implemented only if wanted, separate from the first and second case.
  4. if the task def has ** but no *, throw an error.

P.S. The reason I think **kwargs are not useful is because you either know what dash-flags to expect - in which case you would add them explicitly as named parameters with defaults, or you do not - in which case you want to pass them through as is with as little modifications as possible to the code that does understand them (i.e. call another shell command)

haydenflinner commented 5 years ago

Re: P.S. I'm not sure that that's accurate, I use the *args / *kwargs on a work project in the same way that they're often used on regular Python functions: Wrapping another inv task without knowing/copying its signature, just by passing in args, **kwargs.

But I think you're perfectly right with 2), I'm surprised I didn't try that case when implementing this feature. If you want to give implementing it a stab, I'd merge it pretty quickly after verifying that it worked. I think it should be relatively easy:

        # Flag
        if self.context and (
            # Usual case, --abc-xyz
            token in self.context.flags
            # Hack to allow --args_like_this
            or is_long_flag(token) and translate_underscores(token) in self.context.flags
           # Only when function takes kwargs
            or self.context.eat_all
            and is_flag(token)
        ):

This is the if statement in parser.py which catches flags on a function which takes args or kwargs. Below on line 287 is the line which we fall through to if we fail to accept an argv value as a flag. If you changed eat_all from a boolean to a [None, 'args', 'args_and_kwargs'] enum (representing cases 1-3 of your comment) you should be able to get the behavior that you want by just wiring up that value to the appropriate property of the wrapped task. Though I understand if you'd rather not :smiley: Good luck and thanks for giving the library a shot!

nyurik commented 5 years ago

@haydenflinner thanks for the post. Is there a way to create a small PR with these changes to merge into the main project? This issue is marked with a needs patch status, so clearly maintainers are OK with merging it, assuming we agree on the implementation.

haydenflinner commented 5 years ago

That label was added in 2016, maybe things were still being merged back then, but not now. You could likely cherry-pick / flatten the two or three commits that have been done on this topic from magicinvoke and submit a PR, but if you need it in the next ~6 months (or maybe ever), make sure to submit it to magicinvoke too :stuck_out_tongue: Even things like this or this aren't being merged.

therrj commented 2 years ago

I was running into the issue described here and took a look at the MR here: https://github.com/pyinvoke/invoke/pull/582 and managed to get that working with the 1.6.0 release mostly as is. I had to make a small change which I noted on the ticket.

Is there any way we could get some movement on this?

therrj commented 2 years ago

One last thing to note here in case anyone would like it. I started with @neighthan allow_unknown changes and built on top of it a bit so you can run multiple tasks and it splits up unknown args per task and I also added changes so keyword/positional arguments are supported in addition to the unknown args. So you could do invoke test -f bar unknown_arg1 unknown_arg2

The function signature for the above snippet:

def test(ctx, foo, unknown_args):
    print (foo, unknown_args)

and the output would be: bar ('unknown_arg1', 'unknown_arg2')

And running with multiple tasks:

@invoke.task(allow_unknown=True)
def test2(ctx, foo, unknown_args):
    print (foo, unknown_args)

@invoke.task(allow_unknown=True)
def test2(ctx, bar, unknown_args):
    print (bar, unknown_args)

(Note these commands are a bit shorthand)

invoke test1 -f asd unk1 unk2 test2 -b baz unk3 unk4
asd ('unk1', 'unk2')
baz ('unk3', 'unk4')
dingxiong commented 1 year ago

Any proposal has been merged?

nyurik commented 1 year ago

Any proposal has been merged?

I ended up switching to just