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

Slim handler #548

Merged
merged 5 commits into from
Jan 26, 2017
Merged

Slim handler #548

merged 5 commits into from
Jan 26, 2017

Conversation

mcrowson
Copy link
Collaborator

@mcrowson mcrowson commented Dec 18, 2016

Description

Adds an option to zappa_settings.json for "slim_handler" which defaults to false. When set to true does the following:

  • Creates 2 Zip files for S3. The first is a small handler-only zip (apx 5M) that contains the Zappa handler.py file and zappa's dependencies. The second is the application with all of its dependencies.
  • Copies the versioned application zip to proj_current_project.zip
  • Adds a ZIP_PATH=proj_current_project.zip line to the zappa_settings.py module that is loaded by the handler.
  • When the handler sees the ZIP_PATH= line, it downloads that zip from S3 into /tmp, unzips it, and adds it to path.

Known Issues:

  • The project zip needs to remain in S3 so that the handler can download it as needed. There is not a way to remove old project zip files. They persist in S3. Resolved

GitHub Issues

#510

@coveralls
Copy link

coveralls commented Dec 18, 2016

Coverage Status

Coverage decreased (-0.8%) to 79.965% when pulling cf4d959 on mcrowson:slim_handler into 60cd824 on Miserlou:master.

@bjinwright
Copy link
Collaborator

How long would the files stay in cache?

@mcrowson
Copy link
Collaborator Author

mcrowson commented Dec 18, 2016 via email

… for verionsed project zip files to be removed from S3 as per zappa_settings., metching current functionality.
@coveralls
Copy link

coveralls commented Dec 18, 2016

Coverage Status

Coverage decreased (-1.1%) to 79.625% when pulling 5ad7b99 on mcrowson:slim_handler into 60cd824 on Miserlou:master.

@mcrowson
Copy link
Collaborator Author

@bjinwright I've had keep warm events and been making requests while working on stuff. It has kept the project in /tmp for going on 30 minutes now and hasn't had to re-download from S3. I'll keep checking as it goes on.

@mcrowson
Copy link
Collaborator Author

@Miserlou let me know if there is anything else needed for this.

@kapilt
Copy link
Contributor

kapilt commented Jan 4, 2017

is the goal about keeping the size small ? or supporting large projects over 250mb ?

for keeping the size small which i think is the more universal win, conceptually, this feels PR like the wrong solution and just adds complexity and cold start latency, the right solution imo, would focus on keeping the zappa handler size minimal and getting a better default exclude. the fixed overhead on zappa handler should only be 1mb uncompressed (werkzeug and requestlogger) ~ +310k on the zip

@mcrowson
Copy link
Collaborator Author

mcrowson commented Jan 4, 2017 via email

@smorin
Copy link

smorin commented Jan 4, 2017

Here are the actual limits because it's both 50 and 250 depending on what your exactly talking about. There should be checks for both limits.

I think the ability to get around these limits is another aspect all together as reference in this bug.

There should be a check for what's being copied to tmp because of the /tmp space limits

Ephemeral disk capacity ("/tmp" space)	512 MB

http://docs.aws.amazon.com/lambda/latest/dg/limits.html

AWS Lambda Deployment Limits

Item	Default Limit
Lambda function deployment package size (.zip/.jar file)	50 MB
Total size of all the deployment packages that can be uploaded per region	75 GB
Size of code/dependencies that you can zip into a deployment package (uncompressed zip/jar size)	250 MB
Total size of environment variables set	4 KB
@kapilt
Copy link
Contributor

kapilt commented Jan 9, 2017

so my thought is time spent reducing zappa's overhead from 10mb of the 50mb compressed zip max size to under 1mb, will give more headroom for all apps, and cut the bloat from zappa. the work associated to #556 was to enable that, the work already committed shaved about 7mb off the zip.

for projects that really want bring in the kitchen sink/large bins though this approach is the only that will really work, though it would be nice to see a unit test.

@smorin
Copy link

smorin commented Jan 9, 2017 via email

@narfman0
Copy link
Contributor

Thanks for this! Long term it might be handy to collect packages, measure size, and do this automatically if >50MB. 👍

@mcrowson
Copy link
Collaborator Author

mcrowson commented Jan 12, 2017

I agree. I think for now it seems like something to opt into for a few reasons:

  • Applications may already be taxing their /tmp and a default change like this could have unknown side effects
  • This code isn't as battle tested as the rest of Zappa. All of my tests indicate it works well, but there are always edge cases that we can find later.
  • Currently there is a warning in cli.py if the zip is too large that informs the user about the slim_handler setting.

I looked into adding more tests, but I'm just not sure where. Any suggestions?

# Conflicts:
#	README.md
#	tests/tests.py
#	zappa/cli.py
@coveralls
Copy link

coveralls commented Jan 26, 2017

Coverage Status

Coverage decreased (-1.2%) to 81.153% when pulling f672b82 on mcrowson:slim_handler into 953223b on Miserlou:master.

@Miserlou Miserlou merged commit 4ae771e into Miserlou:master Jan 26, 2017
@mcrowson mcrowson deleted the slim_handler branch May 14, 2017 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants