Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Add simplified docker support #19

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

patcon
Copy link

@patcon patcon commented May 4, 2018

Hi! not sure if this aligns with your vision, but just wanted to put it forward, since the dockerfile has a few bumps as-is.

  • api was proxied in nginx to a host that didn't exist in dockerland
  • alpine linux is waaaaay smaller, to better for local testing when people don't have fast connections available.
  • added docker-compose support

I'm not often a docker user, but given all the ways this repo is used -- api, frontend, processing -- thought it might be helpful :)

Copy link
Contributor

@danvk danvk left a comment

Choose a reason for hiding this comment

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

Hey @patcon, thanks for the PR!!! 🎉Really appreciate your building and running the site.

I'll defer to our Docker experts, @jamadeo and @DOsinga, but the alpine change and the local setup for Docker seem great. How much smaller does the image get when you switch to alpine?

@@ -13,6 +13,6 @@ server {

# proxy the /api to connect to the API server:
location /api {
proxy_pass http://swl-api-server-web:8000;
Copy link
Contributor

Choose a reason for hiding this comment

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

So this isn't visible in Dockerland, but it is visible when this Docker image is deployed on our Kubernetes cluster (where oldto is hosted). We haven't open sourced this API server (yet!) which admittedly makes this a bit confusing. For local development, api.sidewalklabs.com or at an instance of the local dev server (oldtoronto/devserver.py) are good alternatives.

We might want to pass the location of this server in via an environment variable or flag so that we can distinguish "in production" and "local development" more easily.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah man, that sounds good. Maybe based on a check that y'all have in prod, it could just add an /etc/hosts entry? Just chose api.sidewalklabs.com as a quick fix for now. Happy to riff later

@patcon
Copy link
Author

patcon commented May 8, 2018

@danvk alpine gets pretty small :)

~/repos/oldto (improve-docker ✘)✭ ᐅ docker images
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
oldto_webapp        latest              eef5e25f375a        3 days ago          19.5MB
<none>              <none>              e0efa57ee32f        3 days ago          156MB
python              3.6-alpine          8eb1c554687d        2 weeks ago         90.4MB
nginx               1.13.12-alpine      ebe2c7c61055        3 weeks ago         18MB
node                9.8.0-alpine        4a6857f6f75d        7 weeks ago         68.4MB
alpine              latest              3fd9065eaf02        3 months ago        4.15MB
alpine              3.6                 77144d8c6bdc        3 months ago        3.97MB

(base image is only 4MB, and node image was 220MB before)

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

Successfully merging this pull request may close these issues.

2 participants