-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Added the ability to pass in args and kwargs to the zappa invoke
#983
Conversation
The build is not happy! |
zappa/handler.py
Outdated
@@ -336,9 +336,27 @@ def handler(self, event, context): | |||
|
|||
# This is a direct command invocation. | |||
elif event.get('command', None): | |||
if not event['command']: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check is entirely redundant with the previous line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 338 checks for the existence of the key/value of 'command in the dictionary, but doesn't check if that value is a "false" value (None, empty string, etc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if command
is a valid key, thenevent.get('command')
is the same as event['command']
, so it is a check for falsity, with a side effect of returning None
instead of throwing KeyError
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, you are correct, sorry, got my mind on other things...
1 similar comment
I'd still like to see this merged! @Mizell - can you review and test it for your purposes? |
@Miserlou I'll test it out either today or tomorrow. This is great! |
zappa/handler.py
Outdated
whole_function = event['command'] | ||
args = [] | ||
kwargs = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd recommend not going the non-standard way of parsing command line keyword arguments. For most command lines, I'd expect something like zappa invoke "myapp.myfunc" arg1 arg2 --arg3 foo
to provide args as ['arg1', 'arg2']
and kwargs as {'arg3': 'foo'}
.
Of course this is up to you @xuru and @Miserlou, but I think leaving out kwargs for the first iteration would be best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, i've been pretty busy and couldn't get back to this. I do have a ticket this sprint to get back to this, so I can help out tomorrow or this weekend. That said, here is where I left off: I wasn't able to get this working, and I don't really remember what the issue was. I'll take a look and let you know when I can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xuru no worries! And thank you for working on this. I'm going to tinker with it probably tomorrow as well.
Could you please reopen this? |
I'd think it needs to be opened as a bug, not re-opened the original feature creation. |
I'm having the same problem, can't send args or kwargs to functions using |
I also have same problem. Did anyone fix this ? |
Description
Added the ability to pass in arguments to invoked commands from the command line. For example:
$ zappa invoke production 'my_app.my_function' one two three=3 name=bob
Which would get passed to your function in the
event['args']
andevent['kwargs']
dictionary. For example:Would print out
GitHub Issues
Couldn't find a ticket for this, but @Miserlou said in slack: