-
Notifications
You must be signed in to change notification settings - Fork 51
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 production stage to Dockerfile #4053
Conversation
6d6af90
to
a3cf926
Compare
&& curl -L https://github.com/jwilder/dockerize/releases/download/v0.6.1/dockerize-linux-amd64-v0.6.1.tar.gz | tar xvz -C /usr/bin/ \ | ||
&& mkdir -p /tmp/ffmpeg && cd /tmp/ffmpeg \ | ||
&& chmod +x /usr/local/bin/chromedriver | ||
RUN curl https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb -o /chrome.deb |
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.
Is this installed later or only downloaded?
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.
Nvm...I see it now.
Dockerfile
Outdated
# Dev stage for building dev image | ||
FROM ruby:2.5.5-slim-stretch as dev | ||
# Base stage for building final images | ||
FROM ruby:2.5.5-slim-stretch as base | ||
ENV BUNDLER_VERSION 2.0.2 |
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.
Should this be generalized like: https://github.com/avalonmediasystem/avalon/pull/4053/files#diff-3254677a7917c6c01f55212f86c57fbfR16
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.
True
if ENV["RAILS_LOG_TO_STDOUT"].present? | ||
logger = ActiveSupport::Logger.new(STDOUT) | ||
logger.formatter = config.log_formatter | ||
config.logger = ActiveSupport::TaggedLogging.new(logger) |
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.
👍
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.
This looks good to me. I only had a couple questions.
What impact does splitting some of the multi-line RUN commands into their own lines have? I thought you had collapsed them before to cut down on the number of layers.
Cutting down on layers only matters in the final image. For the supporting stages, I sometimes split them up to get granular caching (1 voided cache doesn't ruin others) |
a3cf926
to
29025dd
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.
👍 Squash merge or are the separate commits helpful?
* Restructure Dockerfile, add prod stage * Add build target to docker-compose * Further slim down prod image * Allow logging to stdout in production
No description provided.