Skip to content

Comments

Basic Docker setup#10

Open
derchrisuk wants to merge 2 commits intofroscon:masterfrom
derchrisuk:master
Open

Basic Docker setup#10
derchrisuk wants to merge 2 commits intofroscon:masterfrom
derchrisuk:master

Conversation

@derchrisuk
Copy link

This PR is for a basic setup to run the server instance in a Docker container.
It is using gunicorn for the WSGI server.

@garw
Copy link
Member

garw commented Jan 13, 2019

Sorry for not answering this for so long. I have personally no experience with docker so I am not really sure on how to handle this.

FROM python:2.7
RUN apt update
RUN apt upgrade -y
RUN apt install vim -y
Copy link
Member

Choose a reason for hiding this comment

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

Can you combine this?
RUN apt update && apt upgrade -y && apt clean

And why vim?

RUN apt install vim -y
RUN mkdir /sabot
WORKDIR /sabot
ADD . /sabot
Copy link
Member

Choose a reason for hiding this comment

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

If you resort this, this could save some layers:
ADD . /sabot WORKDIR /sabot

- sabot/settings.py
- sabot/conferenceSettings.py
- sabot/localSettings.py (and sabot/prodSettings.py if you want to distinguish a development and a production version) from their example files (prodSettings.py needs the same parameters)
- Make sure that `PROJECT_ROOT = '/sabot'` is set in sabot/localSettings.py (or sabot/prodSettings.py)
Copy link
Member

Choose a reason for hiding this comment

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

Please mount them using volume mounts or something like this, don't compile them into the docker image.

WORKDIR /sabot
ADD . /sabot
RUN pip install -r requirements.txt
EXPOSE 8000
Copy link
Member

Choose a reason for hiding this comment

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

Why is there no ENTRYPOINT or CMD in here? bash in a container is not how containers should be used

@chrisz100
Copy link
Member

It's been a while but I added some remarks @derchrisuk

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.

3 participants