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

Revised app so that it can be ran in docker. #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

swmcc
Copy link
Member

@swmcc swmcc commented Jan 13, 2020

Created a very quick docker runnable instance of the repo. This is simply to solve people coming to the meetups and not being able to get an instance up and running locally.

We can smart this up later...

  • Make it work
  • Make it faster
  • Make it nicer

etc

Copy link

@pablobm pablobm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for starting work on this! I've been playing with it and I couldn't initially get it to work :-( Eventually I tweaked things here and there and was able to put together a Docker config that I think does the job?

I have created my own branch, based off this one. You can see it at feature/dockerise...pablobm:docker-take-2 These are some highlights:

  • Hex and NPM packages are both included in the image, and cached to a volume. (Note that this means building them twice sometimes.)
  • Separates copying the source code from copying the dependency description files (mix.*, package*.json). This way dependencies are not re-fetched when code changes across image builds.
  • Database data is saved to a volume, and therefore persisted across runs.
  • Automatic page reloads on code changes.

All these changes are inspired by the excellent book Docker for Rails Developers, which I very much recommend! Issues are my fault, successes to be credited to the book. Otherwise, I'm very new to Docker so someone please provide feedback!

@@ -0,0 +1,4 @@
$xport POSTGRES_USER='swm'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? $xport

$xport POSTGRES_USER='swm'
export POSTGRES_PASSWORD=''
export POSTGRES_DB='website_dev'
export POSTGRES_HOST='localhost'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, was this file intended to be committed?


set -e

until psql -h db -U "postgres" -c '\q' 2>/dev/null; do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't get this to work but, after looking around for a bit, I saw that it'll pick up the envvars automatically if they have the names PGUSER, PGPASSWORD, PGHOST, PGDATABASE. Then it works and doesn't need the -h db -U postgres options.

If the envvars change, then they need to change also on other references in the context of the web container (but not the db container, which uses it's own names POSTGRES_*).



* export your env vars via `local.env.template` (this isn't loaded so amend, copy, paste and execute).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could instruct users to do source local.env.template.


RUN apk add postgresql-client
RUN apk add nodejs
RUN apk add --update npm
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we install inotify-tools, we get automatic page reload on code changes.

COPY deps/phoenix deps/phoenix
COPY deps/phoenix_html deps/phoenix_html

RUN mix local.hex --force
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should come before line 10 (which copies the code onto the image). This way, a code change doesn't bust the image cache.


RUN mix local.hex --force

RUN cd assets && npm install
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here. Moving npm install to before copying the code (separating copying the code and copying the package*.json files) would avoid busting the cache on every code change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants