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.

Sign up for GitHub

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

Jump to bottom

Docker #441

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

Docker #441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15f9c72
Adding dockerfile, ignore
karllhughes Mar 31, 2019
4f651f0
Adding Docker instructions
karllhughes Apr 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
node_modules
venv
.github
13 changes: 13 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
FROM nikolaik/python-nodejs:latest
Copy link
Owner

Choose a reason for hiding this comment

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

As mentioned, I'm new to docker, but is there a way we can avoid this dependency without too much hassle? I think I'd much prefer to start with ubuntu and build dependencies on there, or potentially host a base image myself.

Thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe you could start from the official docker python image and then install nodejs.

https://hub.docker.com/_/python

Or start from the official nodejs image and add python, if that turns out to be easier/better.

Copy link
Owner

Choose a reason for hiding this comment

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

I mean, we shouldn't even need nodejs. So the official python image sounds like the way to go! Might be missing some django stuff, but we can install that during setup?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would assume anything you need after python will be installed automatically with pip when running make which is already here. So just swapping out the base image for the python one should work if this is only intended for deployment. I see that the current node dependency is to install devDependencies from package.json for eslint support which is needed for development locally.

Copy link
Owner

Choose a reason for hiding this comment

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

Sounds good. Official python image is the way to go then :)


WORKDIR /app

EXPOSE 8000

COPY . /app

RUN make setup

RUN make

CMD python manage.py runserver 0.0.0.0:8000
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ Inspired by [arkt.is/t/](http://arkt.is/t/Yy53aWR0aD0yZTM7eC5maWxsUmVjdCgxNTAsMT
5. Get back in the main directory (`cd ../.. && make`) and use `make` command (install dependencies and set up database)
6. Continue with the fourth step from **Linux setup**.

#### **Docker**
1. Build the Docker image using the `Dockerfile` at the root directory of this project: `docker build -t dwitter .`.
2. Run the migrations: `docker run -it -v $(pwd):/app dwitter make migrate`.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
2. Run the migrations: `docker run -it -v $(pwd):/app dwitter make migrate`.
2. Run the migrations: `docker run -it -v $(pwd):/dwitter dwitter make migrate`.

3. Create the admin account: `docker run -it -v $(pwd):/app dwitter python manage.py createsuperuser`.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
3. Create the admin account: `docker run -it -v $(pwd):/app dwitter python manage.py createsuperuser`.
3. Create the admin account: `docker run -it -v $(pwd):/dwitter dwitter python manage.py createsuperuser`.

4. Run the app in a Docker container: `docker run -it -v $(pwd):/app -p 8000:8000 dwitter`.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
4. Run the app in a Docker container: `docker run -it -v $(pwd):/app -p 8000:8000 dwitter`.
4. Run the app in a Docker container: `docker run -it -v $(pwd):/dwitter -p 8000:8000 dwitter`.


## Other commands
* `make migrations`
* `make migrate`
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ django-filter==1.1
django-hosts==3.0
django-registration-redux==2.4
django-storages-redux==1.3.2
django==1.11.15
django==1.11.17
djangorestframework==3.7.1
flake8==2.6.2
newrelic==4.2.0.100