-
Notifications
You must be signed in to change notification settings - Fork 23
Add postgres development database via docker compose #393
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
Conversation
brandur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Okay, lemme just preface this by saying that neither Blake nor myself are crazy about Docker in general. I'd make the argument that the amount of configuration and boilerplate necessary here to get tests running on Dockers is fairly incredible compared to the alternative of just running createdb river_test.
And although I don't personally understand why people like to develop anything through Docker (let alone Go, which is far fewer commands to run outside of Docker than inside), I know some people prefer it so I'm not against including this. But just a heads up that if it stops working and becomes unmaintained in the future we may take it out again because I don't think I'd be willing to spend the time figuring out Docker compose to fix it.
Left a couple comments below, but otherwise LGTM!
Makefile
Outdated
| docker compose -f docker-compose.dev.yaml up | ||
|
|
||
| .PHONY: db/down | ||
| db/down: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you name these something like docker-db/up instead? Don't feel strongly about a specific name, but they should include "docker".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went with docker-db/up and docker-db/down
docs/development.md
Outdated
| $ river migrate-up --database-url postgres://localhost/river_dev | ||
| ``` | ||
|
|
||
| ## Postgres with docker compose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the nits here, but lets just make sure to use correct capitalization and punctuation everywhere here:
| ## Postgres with docker compose | |
| ## Postgres with Docker compose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never upset about consistency and proper documentation. Addressed ^_^
docs/development.md
Outdated
| ``` | ||
|
|
||
| ## Postgres with docker compose | ||
| If you decided to use postgres in docker compose, you can skip database migration steps for testing and development |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| If you decided to use postgres in docker compose, you can skip database migration steps for testing and development | |
| Using Docker compose, you can skip the database migration steps for testing and development. |
docs/development.md
Outdated
| ## Postgres with docker compose | ||
| If you decided to use postgres in docker compose, you can skip database migration steps for testing and development | ||
|
|
||
| The database would be accessible through localhost |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this line adds all that much as is. Maybe be specific that the Docker database becomes bound to your local machine at localhost:5432 if that's the case.
Oh yeah, hm, not sure how that wasn't in there. Thanks! |
|
Addressed the feedback.
I work on different projects with their own Postgres database and other dependencies, Docker Compose is usually quite common and is the easiest solution to spin up dev environments without the host system becoming a speghatti of dev tools. I understand the complexity it brings too and why it may not be preferred for simpler setups.
Very understandable! |
|
Awesome. Thanks again! |
While working #390 I needed a local PostgreSQL instance to start the development server and run the tests.
It took me a while to figure out the database setup so I thought I should share this setup and document it.
I also added a "TEST_DATABASE_URL" variable to
env.localto make it easy to run the tests. I recon it can interfere with current workflows, so I'm happy to remove it if necessary.