Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*args and **kwds support in tasks #378

Open
pandey1010 opened this issue Jul 20, 2016 · 24 comments
Open

*args and **kwds support in tasks #378

pandey1010 opened this issue Jul 20, 2016 · 24 comments

Comments

@pandey1010
Copy link

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
Copy link
Member

bitprophet commented Jul 20, 2016

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:

  • Just raise an exception if the user actually triggers the ambiguity; I think the parser can handle this, but it may not be able to, been a while since I was in there.
    • If this is possible, it's probably ideal, since the ambiguity case is likely to be pretty rare.
    • Might want to be a warning instead of an exception, since otherwise there's no way for a user to work around it - i.e. maybe they do have a bunch task, but they are not intending to ever call it on the same command line as mytask - they want the word bunch to be passed to mytask as a regular splat-arg member.
      • Or have a core flag like --warn-about-splat-args?
  • Have the parser know beforehand that a task takes arbitrary args (either by an explicit task-generation parameter, or just by noticing the existence of splat-args in the signature), and have it explicitly disable any subsequent multi-task lookup.
    • This is only good if we assume users always intend to be using the splat-args and aren't actually making a silly mistake.
    • It also violates Zen of Python #12 so I'd probably rather not. Maybe let users enable this with a setting/flag.

@SwampFalc
Copy link

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
Copy link
Member

bitprophet commented Nov 29, 2016

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
Copy link

iago1460 commented Dec 5, 2016

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

@elsimir
Copy link

elsimir commented Apr 28, 2017

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
Copy link
Member

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:

  • The most obvious input format would be "use whatever damn flags you like and it just shows up in **kwargs", i.e. inv mytask --foo=bar --biz=baz, where the task is defined as def mytask(**kwargs).
    • This probably requires a minor overhaul of the parser, would have to see.
    • Might dovetail with the work recently done for --help isn't discoverable / cannot give --help after task names, only before #205 though? Maybe?
    • Extra-credit version would need to handle partial-kwargs (Python 2.7 and up, if not 3-only) i.e. def mytask(foo, bar, **kwargs) - where the parser now has to first look out for "real" arguments, and then treat anything otherwise unparseable as implicit args.
  • If that were somehow too difficult, could possibly do something like "any foo=bar style positional arguments get automatically turned into kwargs", but I really don't like this because it's just going back to ye olde Fabric 1 style custom non-Unix-feeling behavior.

@dblandin
Copy link

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
Copy link
Contributor

haydenflinner commented Sep 23, 2018

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 😄

haydenflinner added a commit to haydenflinner/invoke that referenced this issue Sep 23, 2018
@haydenflinner
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link

dblandin commented Nov 7, 2018

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
Copy link

johnraz commented Jan 6, 2019

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
Copy link
Contributor

@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
Copy link

johnraz commented Jan 7, 2019

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
Copy link
Contributor

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
Copy link

@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
Copy link

nyurik commented Oct 7, 2019

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=[...])
  1. 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
  2. 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.
  3. 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
Copy link
Contributor

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 😃 Good luck and thanks for giving the library a shot!

@nyurik
Copy link

nyurik commented Oct 8, 2019

@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
Copy link
Contributor

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 😛 Even things like this or this aren't being merged.

@therrj
Copy link

therrj commented Dec 8, 2021

I was running into the issue described here and took a look at the MR here: #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
Copy link

therrj commented Dec 13, 2021

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
Copy link

Any proposal has been merged?

@nyurik
Copy link

nyurik commented May 13, 2023

Any proposal has been merged?

I ended up switching to just

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests