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

chore: allow docker compose to start the backend server (plbstl) #4995

Merged
merged 16 commits into from
Feb 28, 2024

Conversation

plbstl
Copy link
Contributor

@plbstl plbstl commented Feb 2, 2024

Description

The Backend section of the Advanced Contribution Guide isn't entirely true. You cannot use docker compose up to start the backend server. The command only starts redis and mongodb services, while having to manually run one of the dev scripts to start the backend server.

This change makes sure that docker compose up in the backend folder also starts the server, making contributions to this project a bit more seamless.

I had to use a separate volume for the node_modules in the backend because of mismatch between where the bcrypt module is built (my PC - MacOS) and where it is being run (Docker - Linux). It creates an error that prevents the server from booting up.

services:
  #...
  api-server:
    #...
    volumes:
      #...
      - /monkeytype/backend/node_modules
    entrypoint: 'bash -c "cd /monkeytype/backend && npm install && npm run dev"'

Only the backend dependencies are installed and built in the Docker container. The script can be a bit more complicated to skip unnecessary npm installs, but this is also okay.

Checks

  • Check if any open issues are related to this PR; if so, be sure to tag them below.
  • Make sure the PR title follows the Conventional Commits standard. (https://www.conventionalcommits.org for more info)
  • Make sure to include your GitHub username inside parentheses at the end of the PR title

@monkeytypegeorge monkeytypegeorge added backend Server stuff frontend User interface or web stuff labels Feb 2, 2024
@plbstl plbstl changed the title chore(docker-compose): allow docker compose to start the backend server (plbstl) chore: allow docker compose to start the backend server (plbstl) Feb 2, 2024
Comment on lines +1 to 6
name: monkeytype
version: "3.8"
services:
monkeytype-redis:
redis:
container_name: monkeytype-redis
image: redis:6.2.6
Copy link
Member

Choose a reason for hiding this comment

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

Whats the difference between the two?

@Miodec
Copy link
Member

Miodec commented Feb 5, 2024

My one issue with this PR is that I personally use Docker to only host the databases, and then run the servers in node myself. Maybe we can have one file for just databases and one for databses and node server?

@Miodec Miodec added the waiting for update Pull requests or issues that require changes/comments before continuing label Feb 5, 2024
@plbstl
Copy link
Contributor Author

plbstl commented Feb 6, 2024

Maybe we can have one file for just databases and one for databases and node server?

Yes, that can work. Maybe use the COMPOSE_FILE env var to determine the file to use

@Miodec
Copy link
Member

Miodec commented Feb 9, 2024

We can't just use multiple files?

@plbstl
Copy link
Contributor Author

plbstl commented Feb 15, 2024

Yes, we're using multiple files. The env var is not necessary but a nice to have. Now thinking about it, it may not even be needed

@Miodec
Copy link
Member

Miodec commented Feb 15, 2024

Yes, we're using multiple files. The env var is not necessary but a nice to have. Now thinking about it, it may not even be needed

I mean multiple files for composing backend:

  • one that only runs the databasees
  • one that runs both databases and the api server

Copy link
Contributor

This PR is stale. Please trigger a re-run of the PR check action.

@github-actions github-actions bot added the Stale Has not been updated in a while label Feb 22, 2024
@plbstl
Copy link
Contributor Author

plbstl commented Feb 24, 2024

Maybe we can have one file for just databases and one for databases and node server?

Yes, that can work. Maybe use the COMPOSE_FILE env var to determine the file to use

Let me elaborate on what I meant by this.

There will be two docker-compose files in the backend. When someone wants to contribute and they run docker compose up, the env var will instruct docker on which file to use. So, there is a default file being used.

Yes, we're using multiple files. The env var is not necessary but a nice to have. Now thinking about it, it may not even be needed

We know by default, docker compose checks for docker-compose.yaml. So the default file that is supposed to be set in the COMPOSE_FILE env var can be named docker-compose.yaml, and the other one named something more specific which will be run with docker-compose -f <filename> up

@github-actions github-actions bot removed the Stale Has not been updated in a while label Feb 24, 2024
@monkeytypegeorge monkeytypegeorge added the docs Related to Markdown files and documentation label Feb 25, 2024
@Miodec
Copy link
Member

Miodec commented Feb 25, 2024

Maybe we can have one file for just databases and one for databases and node server?

Yes, that can work. Maybe use the COMPOSE_FILE env var to determine the file to use

Let me elaborate on what I meant by this.

There will be two docker-compose files in the backend. When someone wants to contribute and they run docker compose up, the env var will instruct docker on which file to use. So, there is a default file being used.

Yes, we're using multiple files. The env var is not necessary but a nice to have. Now thinking about it, it may not even be needed

We know by default, docker compose checks for docker-compose.yaml. So the default file that is supposed to be set in the COMPOSE_FILE env var can be named docker-compose.yaml, and the other one named something more specific which will be run with docker-compose -f <filename> up

In that case, i would move the compose files to a backend/docker and rename them to db-and-server and db-only. Then add npm run docker and npm run docker-db scripts.

@Miodec Miodec mentioned this pull request Feb 26, 2024
@Miodec Miodec merged commit a0416d3 into monkeytypegame:master Feb 28, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Server stuff docs Related to Markdown files and documentation frontend User interface or web stuff waiting for update Pull requests or issues that require changes/comments before continuing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants