iliana / rust-crowbar

Wrapper to simplify writing AWS Lambda functions in Rust (using the Python execution environment)
https://docs.rs/crowbar
Apache License 2.0
197 stars 16 forks source link

handler should probably require FnMut, not just FnOnce #21

Open iliana opened 6 years ago

iliana commented 6 years ago

In this function signature:

https://github.com/ilianaw/rust-crowbar/blob/0b1b1b72308faa6f0c176a8e9f5ef061dea4a951/src/lib.rs#L242-L250

FnOnce does not imply that the provided function can be called multiple times, which is an assumption made by Lambda. Perhaps FnMut is more appropriate?

euank commented 6 years ago

which is an assumption made by Lambda

Does Lambda document this? My naive assumption was that each lambda request / event would end up creating, at a minimum, a new process.

iliana commented 6 years ago

https://docs.aws.amazon.com/lambda/latest/dg/lambda-introduction.html

Any declarations in your Lambda function code (outside the handler code, see Programming Model) remains initialized, providing additional optimization when the function is invoked again. For example, if your Lambda function establishes a database connection, instead of reestablishing the connection, the original connection is used in subsequent invocations. You can add logic in your code to check if a connection already exists before creating one.

In a Python context, I've always taken this to mean that the import code step happens once per execution environment bringup, and that your function gets called whenever an event occurs.

iliana commented 6 years ago

See also https://github.com/ilianaw/rust-crowbar/issues/4 where it was discovered that lazy_static can be used to keep things around between invocations.

iliana commented 5 years ago

@softprops do you have any thoughts about this issue? I'm debating whether I fix this for the 0.3.0 release or not still.

softprops commented 5 years ago

I think this is fine as is I've been using lazy static and thread local https://github.com/meetup/pulley/blob/master/src/lib.rs#L67

In general it's best to initialize expensive ops like this for performance in lambda. For cheap initialization, do it inside the handler and pass state as arguments.

softprops commented 5 years ago

If you're looking to cut a new release I've got one change I'd like to get in. An setup ergonomic issue I recently solved for lando, an extension of crowbar for the http crate https://github.com/softprops/lando/pull/24 this removes the need for users to declare a cpython dependency I'm cargo.toml and lib.rs files by reexporting macros. I've been following along with the rust topics on enabling this https://github.com/softprops/lando/issues/7

softprops commented 5 years ago

I'll try to follow up with a crowbar pull today and you can let me know what you think

softprops commented 5 years ago

Im actually going to open a new issue on this one realizing the context is going to get lost in an unrelated thread :)

softprops commented 5 years ago

carrying on over here https://github.com/ilianaw/rust-crowbar/issues/45