softprops / lambda-rust

🐳 🦀 a dockerized lambda build env for rust applications
MIT License
162 stars 76 forks source link

Keep bootstrap binary #61

Closed medwards closed 4 years ago

medwards commented 4 years ago

This is convenient for local testing because one can skip the unzipping step before using the lambci/lambda image.

medwards commented 4 years ago

Couple of other thoughts:

Users can force the binary name themselves in the [[bin]] section of the manifest. I'm assuming this isn't expected of the user because they might have a workspace producing multiple lambda boostrap binaries?

softprops commented 4 years ago

Users can force the binary name themselves in the [[bin]] section of the manifest.

This is supported but uneccessary

https://github.com/softprops/lambda-rust/blob/7e99c0c3300f7f98183b9f60f49c98118c0b5738/tests/test-func/Cargo.toml#L8

The fact that the binary needs to have a very specific name in the deployment target is an implementation detail, most users don't care about. This is a motivating factor for the managed packaging script.

I'm assuming this isn't expected of the user because they might have a workspace producing multiple lambda boostrap binaries?

That's a good point. Workspaces are supported. Another motivating factor for the current packaging approach.

medwards commented 4 years ago

I can understand that. I could add a subfolder to the release folder where the release artifacts are prepped. Zip file stays where it is but there's still a mountable folder that is clean for docker. How's that sound?

On Sat., Apr. 25, 2020, 15:26 Doug Tangren, notifications@github.com wrote:

@softprops commented on this pull request.

In README.md https://github.com/softprops/lambda-rust/pull/61#discussion_r415062940:

 -i -e DOCKER_LAMBDA_USE_STDIN=1 \

--rm \

  • -v /tmp/lambda:/var/task:ro,delegated \
  • -v ${PWD}/target/lambda/release:/var/task:ro,delegated \

My main concern here is this might not necessarily reflect the behavior in the lambda environment. Since cargo shares on target directory for all workspace members and since more than what's packaged and delivered to the lambda environment lives in the target dir, you are like to run into cases where you'll see one behavior locally and another when running in lambda.

With the current approach you are exposing the exact same artifacts that will be deployed to lambda to your local test environment.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/softprops/lambda-rust/pull/61#pullrequestreview-400393596, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABZK7URSPL3TEMIKN2F73ROLQJPANCNFSM4MQXKLEQ .

medwards commented 4 years ago

Threw in a commit to help with local testing. Helped me catch some dumb mistakes. You can now run: ./tests/test.sh `docker build --quiet .` and test just whats in the local folder. You might have some cleanup to do after.

If I understood [ "$file" != ./bootstrap ] && [ "$file" != bootstrap ] was intended to avoid renaming if the binary was already named bootstrap. Since we're making a new folder for output artifacts this is no longer conditional and I removed it.

I vaguely remember an issue about packaging resources with the binary. If so, I haven't done any thinking on how this would fit in with a solution for resources.

softprops commented 4 years ago

I'll take a look. Note that my turn around time is going to be a bit slow, I just had a baby!

The general idea makes sense to me.

medwards commented 4 years ago

Congratulations! My best wishes to the mother, yourself, and child 🥳🥳

As for turnaround, no worries. Working on this PR means I have a local image that does what I want and that's good enough for now! :)

On Sun., Apr. 26, 2020, 20:21 Doug Tangren, notifications@github.com wrote:

I'll take a look. Note that my turn around time is going to be a bit slow, I just had a baby!

The general idea makes sense to me.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/softprops/lambda-rust/pull/61#issuecomment-619599935, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABZK44VVFTWKVGZERG273ROR3SHANCNFSM4MQXKLEQ .

Veetaha commented 4 years ago

@softprops congratulations with the baby! :D

Regarding the PR, I stumbled with this too, intermediate zipping and unzipping is annoyingly long when all you want is cargo run experience (i.e. build and run the lambda as fast as it is possible). I'd even add a flag to skip making .zip archive entirely if you don't mind @medwards @softprops ?

softprops commented 4 years ago

Let's make that a separate pull @Veetaha.