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

[Proposal] Chalice invoke command #900

Closed
kyleknap opened this issue Jul 15, 2018 · 14 comments
Closed

[Proposal] Chalice invoke command #900

kyleknap opened this issue Jul 15, 2018 · 14 comments
Assignees

Comments

@kyleknap
Copy link
Contributor

kyleknap commented Jul 15, 2018

Overview

With all of the new event types being released into Chalice, it would be great if Chalice had a built-in way to invoke the deployed Lambda function (and possibly a local version). Right now, I am invoking these functions through the console or the AWS CLI, but it would be much more convenient if Chalice had a built-in invoke command that accepts:

  • --name <function-name>: The logical name of the Lambda function in the chalice app
  • --stage <stage-name>: To specify the stage you want to target
  • --payload <value>: A JSON payload to use when invoking the Lambda function

It would then print the result of the invoke to standard output.

Example

Using this deployed app.py:

from chalice import Chalice

app = Chalice(app_name='intro')


@app.lambda_function()
def hello(event, context):
    return {'hello': 'world'}


@app.lambda_function()
def hello_name(event, context):
    return {'hello': event['Name']}

Users would then be able to invoke the deployed Lambda functions manually:

$ chalice invoke --name hello
{"hello": "world"}
$ chalice invoke --name hello_name --payload '{"Name": "Kyle"}'
{"hello": "Kyle"}

Future additions

  • Preformatted event payloads. Not sure how this would look quite yet, but essentially you can just provide the only the important event parameters to chalice invoke (i.e. bucket/key for S3 event or message for SNS event) and chalice will take the parameters and format an event on your behalf based on what the event source is and invoke the function.

  • Local mode. Potentially add a --local flag so this will invoke your Lambda function in your Chalice application locally.

  • --context <context-value>. Add the ability to send a context as well as a payload to invoke the Lambda function.

@jamesls
Copy link
Member

jamesls commented Jul 16, 2018

In general, I like the idea but there's a few edge cases I'm curious about:

  • What's the expected behavior if I try this on a resource that's not an @app.lambda_function()?
  • Payload is required is it not? I think we can shorten it to just be a positional param. I would also expect a common use case to load the payload from a file. I'd almost think that's more common than specifying the JSON inline.
  • We should support -n/--name for the name arg, consistency with chalice logs -n.

I'm also not sure if we want this as a top level command. Thinking through the direction of improved local testing, it seems like putting this under the local command would make the most sense. What about chalice local invoke? We already have a local http server, this issue would add a "import then run your code" mode, and I could see in the future adding optional support for the docker lambda image assuming there's customer interest. How would you see the commands for these new features looking like?

@kyleknap
Copy link
Contributor Author

Just replying back. Let me know what you think:

What's the expected behavior if I try this on a resource that's not an @app.lambda_function()?

It would just accept the raw payload no matter what it is decorated with. So you would have to format the proper event payload if it is hooked up to an event source (i.e. s3 event, sns message, etc.). I think it would be interesting though if we can find a way to do preformatted payload as I mentioned in the initial overview. I'm just not sure how it would look, but I think no matter what it would be great to have the raw payload invoke.

Payload is required is it not?

Actually it is not from the SDK. Using the CLI on this app.py:

from chalice import Chalice

app = Chalice(app_name='hello-lambda')
app.debug = True

@app.lambda_function()
def index(event, context):
    print(event)
    return {'hello': 'world'}

I can run this command:

aws lambda invoke --function-name hello-lambda-dev-index saved-result

and it works fine without a payload. If omitted, an empty dictionary is provided.

I think we can shorten it to just be a positional param. I would also expect a common use case to load the payload from a file. I'd almost think that's more common than specifying the JSON inline.

Yeah I would be fine with it being a positional argument as well. As to a file, I think that is nice especially if we are going to be allowing you to pass raw payloads for non-app.lambda_function() decorated functions. I would still like to have the option to pass it directly to the command line though for a couple of reasons:

  1. It is a little annoying to save the JSON to a file first and then invoke the lambda function when the event payload is something small.

  2. I do this in the tutorial I am building, but if I save a value to an environment/bash variable. Like I do here, I really cannot take advantage of the variable. I would have to echo it to a file and then use that file. That was just only my specific scenario, but I can imagine there are other examples where you want to use a functionality of the shell to construct the payload.

We should support -n/--name for the name arg, consistency with chalice logs -n

Makes sense to me.

What about chalice local invoke?

I think that could make sense if we were just targeting it for local development. However, the primary way I am planning to use it is to invoke the remote lambda function. You can see an example in one of the PR's I was working on for the new chalice tutorial: aws-samples/chalice-workshop#13. So my main concern is that it would be weird to have that under the local subcommand when this will be making a remote invocation. Not sure if that changes your thoughts on that?

@stealthycoin
Copy link
Contributor

  • +1 on loading from a file.

  • The the local and remote versions might be nice to keep separate since they may evolve independently in the future. For example override parameters could be provided to overriding real configured parameters on a per invocation basis to make testing easier. So maybe putting one as chalice invoke and one as chalice local invoke makes sense.

@jamesls
Copy link
Member

jamesls commented Jul 17, 2018

Keeping it as chalice invoke seems fine to me. Given this is a simple wrapper around aws lambda invoke, would you see us adding an equivalent of http [GET/POST] URL? If so would that be a separate chalice command or would we try to make that part of the invoke command?

I do this in the tutorial I am building, but if I save a value to an environment/bash variable. Like I do here, I really cannot take advantage of the variable.

Not necessarily. We could support reading from stdin, which would actually accommodate both use cases. It's also what other tools such as httpie and SAM local do. Maybe that's the simplest thing for now.

@kyleknap
Copy link
Contributor Author

Would you see us adding an equivalent of http [GET/POST] URL? If so would that be a separate chalice command or would we try to make that part of the invoke command?

I could see it being part of the invoke command. Thinking about it more we could expose an --event-type or --type flag where you can specify http, s3, sns, etc and then the payload provided would differ depending on the type provided. Here are some examples:

HTTP
Given the event.json:

{
   "method": "GET",
   "path": "/"
}

You could call the invoke function like this (not sure if we would need to include the -n flag for the api handler):

$ chalice invoke event.json --event-type http

S3
Given the event.json:

{
   "bucket": "mybucket",
   "key": "mykey",
   "eventName": "ObjectCreated:Put"
}

You could call the invoke function like this:

$ chalice invoke event.json --event-type s3 -n my_s3_handler

We could support reading from stdin, which would actually accommodate both use cases.

Using stdin would suit my use case. I am assuming if stdin is specified it would take no positional arguments and read from stdin like this?

$ cat event.json | chalice invoke -n myfunction

I think that would work fine

I would like your thoughts on both of these.

@stealthycoin
Copy link
Contributor

Input

For the input source I like:

$ cat event.json | chalice invoke -n myfunction

I don't think the --event-type or the HTTP specific options are necessary. I'd rather just keep it dead simple and have it invoke the lambda function and pass your specified payload to the function as is. If you want to invoke lambda through API gateway use httpie. If you really want to see what is being sent from lambda to api gateway you can use chalice invoke -n api_handler and pass a payload like this:

{
   "method": "GET",
   "path": "/"
}

Output

Something else to consider is the output. We get back the actual response from lambda, one component of which is a payload streaming body. My thought is we just ignore all the metadata and response keys and write the Payload to stdout. The only exception is if we have a key/value pair: "FunctionError": "Unhandled",. In which case we get back a body that looks like this:

{"errorMessage": "<__main__.LambdaContext object at 0x7fc24750de10> is not JSON serializable", "errorType": "TypeError", "stackTrace": [["/var/lang/lib/python3.6/json/__init__.py", 238, "dumps", "**kw).encode(obj)"], ["/var/lang/lib/python3.6/json/encoder.py", 199, "encode", "chunks = self.iterencode(o, _one_shot=True)"], ["/var/lang/lib/python3.6/json/encoder.py", 257, "iterencode", "return _iterencode(o, 0)"], ["/var/runtime/awslambda/bootstrap.py", 110, "decimal_serializer", "raise TypeError(repr(o) + \" is not JSON serializable\")"]]}

I'd like to print this information like a standard python error/traceback to stder. I think that would make for the cleanest experience debugging a misbehaving lambda function.

@jamesls
Copy link
Member

jamesls commented Jul 17, 2018

chalice invoke event.json --event-type http

I'm not really sure what that means. You have a file and an event type?

I'm with @stealthycoin on this, I think just keep it simple. If you pipe something to stdin, that's the payload, othewise there's no payload sent. Nothing else for payloads (for now).

I think there's enough to go ahead and send a PR. I think we can work out the remaining details on the PR.

@kyleknap
Copy link
Contributor Author

Ok cool. I was just trying to brainstorm on how we could expand on it to handle different invocation types and if we would want to expand on it. Was not trying to suggest that we do it. I like the interface that we have now (after what @stealthycoin suggested).

@stealthycoin
Copy link
Contributor

I have a POC already so I will update it to match the proposed interface and send a PR.

@stealthycoin stealthycoin self-assigned this Jul 17, 2018
stealthycoin added a commit to stealthycoin/chalice that referenced this issue Jul 17, 2018
stealthycoin added a commit to stealthycoin/chalice that referenced this issue Jul 17, 2018
@stealthycoin
Copy link
Contributor

What should we do about the return codes. There are multiple ways it can fail.

  • It couldn't invoke the function for one reason or another.
  • A lambda error occurred, causing botocore to raise an exception.
  • An unhanded error occurred inside the lambda function.

@kyleknap
Copy link
Contributor Author

kyleknap commented Jul 18, 2018

As to return code, how about the following?

0 - Successful invoke
1 - Invoked but unhandled error
2 - Invoke() called but botocore error returned
255 - Just did not invoke at all from like a wrong function name

@jamesls @stealthycoin I think it is useful to be able to differentiate between an error being encountered in the lambda function and not being invoked/botocore error

@jamesls
Copy link
Member

jamesls commented Jul 18, 2018

I would check to be consistent with the other commands. We should use the same RC when we provide a function name that doesn't exist. Assuming that matches the above, it looks good to me.

@stealthycoin
Copy link
Contributor

For consistency with the other commands I went with:

  • 0 Success
  • 1 Error due to code bug or misconfiguration (botocore error, or unhandled error)
  • 2 Resource not found, this matches chalice url when it isn't deployed it returns 2

@stealthycoin
Copy link
Contributor

Merged in #901

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

3 participants