Skip to content
This repository has been archived by the owner on Jul 6, 2020. It is now read-only.

Change the dev instructions to use docker-compose #13

Merged
merged 2 commits into from
Nov 12, 2017

Conversation

phildini
Copy link
Member

@phildini phildini commented Nov 6, 2017

Hopefully this is self-explanatory. I tested this locally, and it is way easier than the old install instructions, as well as hopefully being closer to the prod setup.

@phildini
Copy link
Member Author

phildini commented Nov 6, 2017

This closes #12 as well.

version: '3'

services:
db:

Choose a reason for hiding this comment

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

This one is conflicting with the postgres installed in my machine.
Would it make sense to use an alternative port for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

wat. how can something on docker conflict with your local machine? Is docker auto-exposing ports?

Choose a reason for hiding this comment

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

hm yea, nvm, it doesn't make sense, i misread the error message (sorry, i had had too much wine yesterday)!
The problem I got was the first time i tried docker-compose up the database wasn't up yet, so Django app didn't start saying:

web_1  | django.db.utils.OperationalError: could not connect to server: Connection refused
web_1  | 	Is the server running on host "db" (172.18.0.2) and accepting
web_1  | 	TCP/IP connections on port 5432?

It worked the second time around, because by then the db was already up.

Maybe we should mention this in the README?

Or possibly install netcat on the image and do a bash loop to wait for the TCP port to be listening, like shown here: https://stackoverflow.com/a/31753726/149872

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be fixed now.

Copy link

@eliasdorneles eliasdorneles left a comment

Choose a reason for hiding this comment

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

Thanks, just tested and works fine now on first try!
Looks great to me! 💯

@phildini phildini merged commit 0e707b1 into beeware:master Nov 12, 2017
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.

None yet

3 participants