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.

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

feat: document how to run postgres in docker #266

Merged
merged 3 commits into from
Aug 19, 2022
Merged

Conversation

dennis-campos
Copy link
Contributor

Docker configuration to bison app.

Docker is set up to run postgres locally. If you want to run the entire app, please refer to the documentation.

Changes

  • Add Docker documentation
  • Add Dockerfile, .dockerignore, and docker-compose.yml
  • Add the files to copyFile.js to generate the proper files when running a bison app.

Checklist

  • Requires dependency update?
  • Generating a new app works

Copy link
Contributor

@kgajera kgajera left a comment

Choose a reason for hiding this comment

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

Here are my thoughts on this:

  • Adding a docker-compose.yml to the template that does not include all services feels incomplete to me. This is not what I would expect if I was developer trying to use it- this might be based on my own experience, but this is how I've seen other template repositories work, for example: https://github.com/graphile/starter. This provides an easy for a way developer to get started without needing to worry about their local setup.
    • For me, the main question is, how many developers would actually utilize a database only docker compose file enough to make it worth adding to the template? Or is it just adding bloat to the template?
  • I am in favor of not adding any of the docker related files to the template, but instead expand the documentation that Dennis added for how to set up a docker-compose.yml, a development Dockerfile, and a link pointing to how to setup a production Dockerfile: https://github.com/vercel/next.js/blob/canary/examples/with-docker/Dockerfile
  • We could add docs similar to Blitz: https://blitzjs.com/docs/postgres. Note how they are handling the test database through the docker compose.

docs/docker.md Outdated
DATABASE_URL="postgres://dev:dev@postgres/dev?schema=public"
```

Somtimes your NextJs will not run due to postgres still booting up. You can add `connect_timeout=300` to the database URL. If you continue to have issues, you can press the `start` button in the docker application.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be a problem if you use depends_on: https://docs.docker.com/compose/compose-file/compose-file-v3/#depends_on

docs/docker.md Outdated

The following configuration will help you run NextJS & Postgres.

Make sure `@prisma/client` is on version: `3.13.0`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this needs to be on version 3.13.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will eliminate this. I was writing this when working on node:alpine Prisma does not work well on < 3.13.0

The error was related to running node:alpine. It will not apply if we are using node:lts

When following NextJS docs they are using node:alpine (https://github.com/vercel/next.js/blob/canary/examples/with-docker/Dockerfile). Running Prisma and alpine has some issues and upgrading made it work:

prisma/prisma#13218 (comment)

@mthomps4
Copy link
Contributor

mthomps4 commented Jul 5, 2022

@kgajera

I am in favor of not adding any of the docker related files to the template, but instead expand the documentation that Dennis added for how to set up a docker-compose.yml, a development Dockerfile, and a link pointing to how to setup a production Dockerfile

I think I'm in the same thoughts here -- If we don't have a complete Docker solution, I would opt for Docs or links to Docs.

I still think it's a good idea for us to figure out a Docker Deploy strategy but this seems more for local dev.

Copy link
Contributor

@cullylarson cullylarson left a comment

Choose a reason for hiding this comment

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

A few thoughts in the comments. Also, did you manage to get testing working with this setup? Do we need to make any changes to accommodate that?

docs/docker.md Outdated

To shut down docker, run `docker-compose down`

## Using NextJS & Postgres with Docker
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to be consistent with Next's official name, it's Next.js. Same throughout.

docs/docker.md Outdated

## Running the Database

To run postgres with docker, please add the following information to your `env` & `env.local` file.
Copy link
Contributor

Choose a reason for hiding this comment

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

...your .env and .env.local file

@@ -0,0 +1,15 @@
# Default configuration to run your NextJs app locally:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to use a Dockerfile for this? Could we just mount the entire ./ folder (excluding .data and likely a few other folders) in docker-compose.yml and run directly from there?

POSTGRES_DB: dev
restart: always
volumes:
- ./.data/postgres:/var/lib/postgresql/data
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the .data/postgres folder idea; that it leaves room for other data folders 👏

postgres:
container_name: postgres
ports:
- 5432:5432
Copy link
Contributor

@cullylarson cullylarson Jul 5, 2022

Choose a reason for hiding this comment

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

I'm wondering if we should change the port. If the user is running a local version of Postgres, this would fail.

Comment on lines 1 to 2
node_modules
/.data
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any more folders that should be added here? Basically you'd want to exclude anything that isn't needed to run the app (e.g. tests). Ideally all of the app code would be in one folder (e.g. src) and we could just mount that folder in docker and not need a .dockerignore file, but Bison's organized a bit differently.

@cullylarson
Copy link
Contributor

cullylarson commented Jul 5, 2022

I think we need to settle the question of whether we're doing this for dev or for production. If it's dev-only, even if we run the app inside Docker, we don't need the Dockerfile. And, actually, for production we probably don't either. If users are going to roll Docker in production, they'll need to plan their own setup anyway. It might not look anything like what we include. Like Kishan and Matt mentioned, I think linking to some docs for a production Docker setup would be better and not documenting that at all on our end (because, even with docs, we can't hit everyone's production setup and we'd need to keep those docs up to date).

  • Adding a docker-compose.yml to the template that does not include all services feels incomplete to me. This is not what I would expect if I was developer trying to use it- this might be based on my own experience, but this is how I've seen other template repositories work, for example: https://github.com/graphile/starter. This provides an easy for a way developer to get started without needing to worry about their local setup.

    • For me, the main question is, how many developers would actually utilize a database only docker compose file enough to make it worth adding to the template? Or is it just adding bloat to the template?

I use a Postgres-only docker-compose setup on a lot of projects. There are a few benefits:

  1. You don't need to install and run Postgres locally and it doesn't need to be part of the onboarding process (just docker-compose up -d). Also, one less daemon running on my machine is always a plus.
  2. You don't need to add a new account to Postgres for each new project (just docker-compose up -d).
  3. It makes project backups a lot easier since your data folder is in your project folder. This is often useful if you're going to do a weird mutation and aren't sure how it will go, or if you're working on a feature but want to pull and review a PR where the database has been changed but you don't want to lose your local data (maybe because it's in just the right state for what you're working on; you can just cp -R .data ~/temp/my-project-data-before-changes.

We discussed this in Discord, but there are a few downsides to running a Next app in Docker:

  1. It's slooooooow. It takes a lot longer for the hot rebuild/reload to happen. You end up waiting around to see your changes show up.
  2. You end up spending time resolving a lot of "how do I connect my app to X or X to my app?" and "what should be env variables be?' issues. They only pop up here and there, but they're annoying and sometimes you have to do some super weird stuff to get everything working together.
  3. You'll also need to run your tests inside Docker and it's even more painfully slow. And the "how do I connect X" and env issues are even worse for testing.
  4. Almost all of the issues I've run into with upgrading Docker have to do with file watching. Anything that watches files for changes (Next, Jest, etc) uses something like Watchman or Chokidar. And I don't know why, but so often I'll have that working just fine, I'll upgrade Docker, and it breaks; those libraries no longer see file changes. You have to downgrade Docker, wait for the next version, and hope it fixes the issue (and downgrade again if it doesn't). Or sometimes it'll just stop working and you'll need to restart Docker. And the errors you'll see don't immediately make you think "I need to restart Docker".

I've worked on a lot of projects where we wanted to "Dockerize all the things!!" in dev and it honestly ends up being a headache (even just because of the bad DX from the slow refresh). I've settled on this: Run node locally, run anything that can run on node outside of Docker, and run everything else in Docker. Yes, managing Node versions can be a headache too, but the tooling around that has gotten so much better; you can specify the node version in code and use your node version manager of choice to install/run the correct one.

@cullylarson
Copy link
Contributor

I forgot to mention: Great work on the documentation, working through the setup, and making good decisions around what to include and not to include, @dennis-campos!

@kgajera
Copy link
Contributor

kgajera commented Jul 6, 2022

I think we need to settle the question of whether we're doing this for dev or for production.

We were aiming for local development set up here.

I use a Postgres-only docker-compose setup on a lot of projects. There are a few benefits:

To me this seems more like a preference on how to run Postgres. I'm questioning whether there is a reason to include it in the template if there's not a clear majority who will run Postgres this way. That's why I like Blitz's approach on adding docs (https://blitzjs.com/docs/postgres) and that can be decided by the project.

We discussed this in Discord, but there are a few downsides to running a Next app in Docker:

Agree with your points to some extent. My main motivation for wanting to include a docker compose that supports running the app that "just works" was to lower the barrier of entry for someone trying out the template.

Also noting that our CLI currently prompts the user for database information when creating an app which I think adds another layer of confusion for a first time user who wants to run in Docker.

@cullylarson
Copy link
Contributor

I use a Postgres-only docker-compose setup on a lot of projects. There are a few benefits:

To me this seems more like a preference on how to run Postgres. I'm questioning whether there is a reason to include it in the template if there's not a clear majority who will run Postgres this way. That's why I like Blitz's approach on adding docs (https://blitzjs.com/docs/postgres) and that can be decided by the project.

Good point. Maybe just documenting options is best.

We discussed this in Discord, but there are a few downsides to running a Next app in Docker:

Agree with your points to some extent. My main motivation for wanting to include a docker compose that supports running the app that "just works" was to lower the barrier of entry for someone trying out the template.

That's a helpful clarification. It does sound appealing. Though I wonder if it sets a bad example since running the dev app in Docker may not be a great setup. We're inviting new users into a bit of a painful developer experience.

Also noting that our CLI currently prompts the user for database information when creating an app which I think adds another layer of confusion for a first time user who wants to run in Docker.

True, that is confusing. If we include a Docker setup, we'd probably want to update that.


So are we reaching a consensus that documenting some Docker setup options is the best move?

docs/postgres.md Show resolved Hide resolved
docs/postgres.md Outdated Show resolved Hide resolved
docs/postgres.md Show resolved Hide resolved
docs/postgres.md Outdated Show resolved Hide resolved
docs/postgres.md Outdated Show resolved Hide resolved
docs/postgres.md Show resolved Hide resolved
Copy link
Contributor

@cullylarson cullylarson left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for iterating and sticking with this one, Dennis.

@cullylarson cullylarson changed the title feat: add docker configuration to bison feat: document how to run postgres in docker Aug 19, 2022
@cullylarson cullylarson merged commit 43a8083 into canary Aug 19, 2022
@cullylarson cullylarson deleted the feat/add-docker branch August 19, 2022 22:59
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.

4 participants