-
Notifications
You must be signed in to change notification settings - Fork 293
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
Add dockerFile option #128
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.
Magnifique! I added 2 more related issues to your description 😄
Limitations:
- build every time is fine since docker's caching should make it fast if nothing has changed.
- huh. Is that a
docker build -f
limitation?
Open questions:
- A warning in that scenario would be great 👍
- I won't reject this because of those limitations, but I'm not sure how to mitigate them either.
lib/pip.js
Outdated
this.serverless.cli.log(`Docker Image: ${this.options.dockerImage}`); | ||
let dockerImage = this.options.dockerImage; | ||
if (this.options.dockerFile) { | ||
this.serverless.cli.log(`Building custom docker image from ${this.options.dockerFile}...`); |
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.
Could you move this docker build logic to a new lib/docker.js
module?
Thanks for your feedback @dschep!
Let me know what you think. |
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.
Looks fantastic! I'll make a release after work when I can test it.
Define the
dockerFile
option that is mutually exclusive with thedockerImage
option.If
dockerFile
is provided, it must be the name of the Dockerfile file, placed in the current directory.When building requirements, a Docker image is built based on the provided Dockerfile, and requirements are then built using this image.
Limitations:
Open Questions:
dockerizePip
is false but any docker related options are provided, no warning are raised. Should I fix it?Related issues: