rust-crowbar icon indicating copy to clipboard operation
rust-crowbar copied to clipboard

handler should probably require FnMut, not just FnOnce

Open iliana opened this issue 8 years ago • 9 comments

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?

iliana avatar Oct 30 '17 02:10 iliana

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.

euank avatar Oct 30 '17 04:10 euank

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 avatar Oct 30 '17 05:10 iliana

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 avatar Oct 30 '17 05:10 iliana

@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.

iliana avatar Oct 02 '18 15:10 iliana

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 avatar Oct 02 '18 16:10 softprops

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 avatar Oct 02 '18 16:10 softprops

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

softprops avatar Oct 02 '18 16:10 softprops

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

softprops avatar Oct 02 '18 18:10 softprops

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

softprops avatar Oct 02 '18 18:10 softprops