-
Notifications
You must be signed in to change notification settings - Fork 363
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
Update/deploy a lambda using a Docker container image #967
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I cant merge this, it does look quite good to me. I did just have the one small stylistic thought while reading through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering when Zappa would support those new AWS Docker features!
Some non-functional changes can be removed -- I think your editor may have accidentally modified some docstrings (unless you meant to do that).
Nice catch on the docstring changes @Quidge. The blank lines in the docstrings should be there. PEP 257 docstring formatting specifies using blank lines to break up multi-line docstrings the way they are in the code currently. https://www.python.org/dev/peps/pep-0257/#id17 Can you touch this up as well @ian-whitestone? |
@techdragon @Quidge I'm just waiting on a final review from @jneves. I will fix those up together with any other changes requested by @jneves |
As someone about to get started with Zapper I just want to say a big thank you for doing this. Support for Docker was a bit of a sticking point so I'm really thankful you've spent the time to add it and PR it @ian-whitestone |
@jneves do you think we'll be able to move this forwards? |
By the way, in the example on your blog, in the Dockerfile you copy in the zappa handler, but I think you can avoid this by referencing the handler like this? CMD [ "zappa.handler.lambda_handler" ] |
One gotcha (sort of) I found with the docker approach is if you use |
Update on this. So it seems like for now, copying in the handler is the best route. |
@ian-whitestone good work, we are looking forward for this feater |
81128e4
to
1329900
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI @mcrowson I ran black zappa tests
and it reformatted a bunch of test files...so this PR will include some changes un-related to the docker functionality
1329900
to
cd1f748
Compare
Had a conversation on Slack with Ian, putting summary here for posterity. First we're working through an issue with Travis CI where they are not running our tests, so hopefully that will be addressed soon and we can see all of the tests passing. The second is I have a hard time mixing arguments via the CLI and arguments via our config file. I made #988 to capture an enhancement to allow for any of them. I asked Ian to implement this feature configuration through the settings file instead of a CLI argument. CI/CD systems could use something like sed to update the config file if the tag changes and is not statically set. Edit: After more analysis/discussion we think that this dockerfile passing is similar to the zip argument. Therefore this approach is fine for now. I'm good to approve once this passes CI. @jneves any other thoughts on this one? |
closed/reopened to try and trigger Travis. Nothing to see here folks. |
cd1f748
to
5a3220c
Compare
cf975cb
to
2aa7cdb
Compare
2aa7cdb
to
5b695e7
Compare
FWIW I've been using Ian's fork for a good few weeks now and not ran into any issues so far deploying using Docker. Anecdotal I know, but just thought I'd put it out there :D Thanks again for adding this to Zappa @ian-whitestone |
It works like a charm even with public.ecr.aws/lambda/python:3.8
|
In case someone wants to migrate their zip-based project to a docker image, remember to remove |
Migrated from Miserlou/Zappa#2192
Background
As discussed in #2188, AWS recently announced container image support for AWS Lambda. This means you can now package and deploy lambda functions as container images, instead of using zip files. The container image based approach will solve a lot of headaches caused by the zip file approach, particularly with file sizes (container images can be up to 10GB) and the dependency issues we all know & love.
What does this PR do?
zappa update
andzappa deploy
commands accept a newdocker_image_uri
parameter.docker_image_uri
must point to an image in Elastic Container Registry (ECR) that complies with these guidelines. Instructions on how to configure this docker image will be provided in an accompanying blog post.save-python-settings-file
CLI command, which will be used to generate & save thezappa_settings.py
file that must be included in your docker imageReproducible example
See this repo which contains an example
Dockerfile
that can be used for testing.Discussion points for reviewers
zappa update,
zappa deploy,
zappa rollback` commands for both creating from a zip file (traditional method) and from a docker image (new method)save-python-settings-file
command, wait until lambda is ready function, etc.) into separate PRsFuture Work & Next Steps
zappa deploy
, you need to first create your own docker and then deploy.zappa rollback
, see this comment for details