-
Notifications
You must be signed in to change notification settings - Fork 1
Docker and Docker Compose support #34
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -213,9 +213,24 @@ def modify_npm_scripts | |
"Capybara.default_max_wait_time = 3\n" | ||
end | ||
|
||
react = options[:webpack] == "react" | ||
|
||
# Docker | ||
copy_file "templates/Dockerfile", "Dockerfile" | ||
|
||
if react | ||
copy_file "templates/Dockerfile-assets", "Dockerfile-assets" | ||
copy_file "templates/.env.docker/.env.docker-webpack", ".env.docker" | ||
copy_file "templates/.env.docker-assets", ".env.docker-assets" | ||
copy_file "templates/docker-compose.yml/docker-compose-webpack.yml", "docker-compose.yml" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The way I organized the files that are different for React and non-React is:
I think this is reasonable, but I could change it if there's already a standard in place. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Granted, this is not reflected in the current directory structure of Perhaps we could start that by having a directory There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense to group the files, and to do that in another PR so the locations of all the files are consistent. I also think before doing that it would be good to figure out #16 since testing that everything works with and without Docker and with and without React takes a long time There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough. #16 has been my white whale since starting the project. I agree with your thoughts though. |
||
else | ||
copy_file "templates/.env.docker/.env.docker-standard", ".env.docker" | ||
copy_file "templates/docker-compose.yml/docker-compose-standard.yml", "docker-compose.yml" | ||
end | ||
|
||
# Retrieve all gems, set up git, display next steps | ||
after_bundle do | ||
if options[:webpack] == "react" | ||
if react | ||
install_js_deps | ||
add_js_files | ||
modify_npm_scripts | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
WEBPACKER_DEV_SERVER_HOST=0.0.0.0 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using environment variables to configure the Webpack Dev server is a new feature merged just a few days ago. Previously you had to change the You still can't use command line arguments, because why would you need to use command line arguments when you can just edit a yaml file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the convention you have, should this be in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are three .env files involved: The first two are copied as The third one is copied as |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
DATABASE_HOST=db |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
DATABASE_HOST=db | ||
WEBPACKER_DEV_SERVER_HOST=assets |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
FROM ruby:2.4.2 | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm assuming you've tested this w/spinning up a postgres DB with some data in it and such with it working fine? I only ask because EEC's Dockerfile has some stuff about setting up keys for postgres, which I can't claim to understand why it's needed, but figured I'd toss it out there. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if what that code is for or whether it's currently necessary in EEC Web. I commented it out locally and was able to build and run the app successfully. |
||
# use a newer version of Node to use Yarn | ||
RUN curl -sL https://deb.nodesource.com/setup_8.x | bash - | ||
|
||
# nodejs: Rails needs a JS runtime | ||
# cmake: required by Rugged, dependency of Pronto | ||
RUN apt-get update -qq \ | ||
&& apt-get install -y -qq nodejs cmake | ||
|
||
RUN npm install -g yarn | ||
|
||
RUN mkdir /app | ||
WORKDIR /app | ||
|
||
ADD Gemfile /app/Gemfile | ||
ADD Gemfile.lock /app/Gemfile.lock | ||
RUN bundle install | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. I haven't tested Chrome driver. I think there's potentially enough work getting that running that it should be a separate issue. |
||
ADD . /app |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
FROM ruby:2.4.2 | ||
|
||
# use a newer version of Node to use Yarn | ||
RUN curl -sL https://deb.nodesource.com/setup_8.x | bash - | ||
|
||
# nodejs: Rails needs a JS runtime | ||
# cmake: required by Rugged, dependency of Pronto | ||
RUN apt-get update -qq \ | ||
&& apt-get install -y -qq nodejs cmake | ||
|
||
RUN npm install -g yarn | ||
|
||
RUN mkdir /app | ||
WORKDIR /app | ||
|
||
ADD Gemfile /app/Gemfile | ||
ADD Gemfile.lock /app/Gemfile.lock | ||
RUN bundle install | ||
|
||
ADD package.json /app/package.json | ||
ADD yarn.lock /app/yarn.lock | ||
RUN yarn install | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this file should only be the above 3 lines (as everything else looks the same as the Then we could copy over That way, if we need to change anything here, like the ruby version, we only have to do it once and it'll be used by both files for future apps generated from the template. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there could be a lot if churn in this file. In theory you shouldn't really need Ruby to run the development. The way Rails and Webpacker is currently written, you have to have the full Rails app installed with all of its gems in order to call the dev server, which is a wrapper around a JavaScript library. This makes a lot of sense when you're running the server locally -- you already have Rails and everything installed, but it doesn't work as well with Docker Compose. I can imagine a future where this could be changed to a nodejs image that doesn't have to know anything about Rails, but Webpacker is something I'm not that familiar with. At this point I don't know enough to know whether having the files completely separate or not will end up being easier to manage. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough. Let's keep this as-is for now, get some experience with using it on new projects/various updates, and figure out what feels best in terms of managing it over time. What's here is simple (in a good way) and that may very well be best. |
||
|
||
ADD . /app |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
version: '3' | ||
services: | ||
db: | ||
image: postgres | ||
web: | ||
build: . | ||
command: bash -c 'rm -f tmp/pids/server.pid && bundle exec rails s --port 3000 --binding 0.0.0.0' | ||
volumes: | ||
- .:/app | ||
ports: | ||
- 3000:3000 | ||
depends_on: | ||
- db | ||
env_file: | ||
- .env.docker |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
version: '3' | ||
services: | ||
db: | ||
image: postgres | ||
assets: | ||
build: | ||
context: . | ||
dockerfile: Dockerfile-assets | ||
command: yarn start | ||
volumes: | ||
- .:/app | ||
- /app/node_modules | ||
env_file: | ||
- .env.docker-assets | ||
web: | ||
build: . | ||
command: bash -c 'rm -f tmp/pids/server.pid && bundle exec rails s --port 3000 --binding 0.0.0.0' | ||
volumes: | ||
- .:/app | ||
ports: | ||
- 3000:3000 | ||
depends_on: | ||
- db | ||
- assets | ||
env_file: | ||
- .env.docker |
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.
Thanks for the documentation. This has led me to create #35 to handle separately - which basically says we should take all this information and populate it in the README of the projects we're creating from this template as well, along with other setup info.