Skip to content
This repository has been archived by the owner on Jun 13, 2019. It is now read-only.

handler should probably require FnMut, not just FnOnce #21

Open
iliana opened this issue Oct 30, 2017 · 9 comments
Open

handler should probably require FnMut, not just FnOnce #21

iliana opened this issue Oct 30, 2017 · 9 comments
Milestone

Comments

@iliana
Copy link
Owner

iliana commented Oct 30, 2017

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 iliana added this to the 0.3.0 milestone Oct 30, 2017
@euank
Copy link
Contributor

euank commented Oct 30, 2017

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

iliana commented Oct 30, 2017

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

iliana commented Oct 30, 2017

See also #4 where it was discovered that lazy_static can be used to keep things around between invocations.

@iliana
Copy link
Owner Author

iliana commented Oct 2, 2018

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

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

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 softprops/lando#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 softprops/lando#7

@softprops
Copy link
Contributor

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

@softprops
Copy link
Contributor

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

@softprops
Copy link
Contributor

carrying on over here #45

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

No branches or pull requests

3 participants