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

Add devcontainer #911

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

Conversation

alexwaibel
Copy link
Contributor

@alexwaibel alexwaibel commented Feb 22, 2024

We already encourage users to use docker to spin up containers for minio and postgres so I figured it could be useful to extend these configs such that devs are able to spin up the entire development environment in docker. This is completely optional, but hopefully this can help newer devs onboard more quickly and avoid compatibility issues (for example incorrect node versions).

This config can also be utilized as a basis for running the project in places like CodeSandbox and GitHub Codespaces, which can sometimes be useful for sharing local builds, quickly spinning up branches and PRs in an env for testing, or developing on hardware like tablets where you might only have access to a browser. Since we utilize multiple containers, some extra steps found here are needed.

Setup steps for the repo when running in a devcontainer on your local machine:

  1. Ensure docker, git, and VS Code are installed
  2. Install the VS Code Dev Containers extension
  3. Clone the project
  4. Open the project root directory in VS Code
  5. Run Dev Containers: Reopen in Container from the Command Palette (F1) and select Momentum Mod API
  6. VS Code with then spin up the containers, reload the window, and connect to the container running the backend.
  7. Once you see the files appear in the file explorer, open .env and add your STEAM_WEB_API_KEY as described here
  8. Use the Command Palette (F1) and run Rebuild Container
  9. Once you see the Nest application successfully started log in the output, open a new VS Code window using File > New Window and open the project root in that new window
  10. Run Dev Containers: Reopen in Container from the Command Palette (F1) and select Momentum Mod Website
  11. Wait until you see the log Local: http://localhost:4200/ and open that address in your browser and you should see the Momentum Mod website. You should now have everything you need to contribute!

Once this is merged into main, we can use the following link instead of steps 2-5:
https://vscode.dev/redirect?url=vscode://ms-vscode-remote.remote-containers/cloneInVolume?url=https://github.com/momentum-mod/website This automatically installs the Dev Containers extension, clones the repo, and opens the project in the container.

Other editors with Dev Container support can be used instead of VS Code can be used although I haven't tested these. See the list of supported editors here

Troubleshooting:

  • If for any reason you hit an error and the container exits, you can use the Command Palette (F1) and run Rebuild Container
  • If you see spinners on the website and Unauthorized errors in the API output, try hard refreshing the web page
  • To stop the containers you can open the project root in a terminal on your host machine and run docker compose down

@tsa96
Copy link
Member

tsa96 commented Feb 23, 2024

What's required between merging this, and getting Github Codespaces/CodeSandbox working?

If this is purely for local development, this seems to just overcomplicate things. I agree it's worth being explicit about minimum Node version and resolving the issue with .dockerignore the Postgres mount is nice, but those are both things that can be solved without introducing a new technology like devcontainers into the mix. Keep in mind that the new Nx setup is less than a year old and myself and others agree it's a major improvement on the old approach. For local development I think we should stick to that and resolve any setup issues that come up, rather than diverging into managing two separate dev setups.

That being said, I'm happy to work towards a fully remote setup. My question is, why do we need to merge this into main to keep working on the Codespaces stuff? Is it not possible to do the same work off of your fork? I'm a little wary of merging something relatively useless only just as a stepping stone for future PRs. On one hand there's not much harm in merging this now, and if you end up disappearing (not saying you will, be it's unfortunately quite common with devs on this project) I could just remove it. One the other hand, the multi-container Codespaces stuff sounds a little dicey, and I worry that merging this now may make it harder to just cut our losses and give up with it in the future, or it may end up snowballs into a huge time-sink for you.

Not that I don't greatly appreciate this work by the way, increasing accessibility for new devs is hugely important for us and this shows a lot of promise! I just want to get a clear picture of what the path forward for this is before merging.

@alexwaibel
Copy link
Contributor Author

I think there's not a lot left to get codespaces working with this, just need to go through these docs and try it out myself and add a few scripts from there to the devcontainer setup. I'm fine continuing that before we consider merging this. I can certainly work off my fork for that.

The big draw of using a devcontainer locally for me is that I don't need to install any dependencies on my local system beyond docker and vs code. So if I work on multiple node projects at the same time that have different version requirements or conflicting ports I don't have to remember to switch back and forth. The number of global dependencies for this project is pretty low (node and nx right now) but running the entire project in a container eliminates the need for these entirely and ensures devs are on exactly the same version so we have a consistent and reproducible dev environment. Granted, I can keep using this myself without having it checked into the repo, but I figured it could also be a useful option for other devs as is.

For now I'll plan to add codespaces stuff to this PR though.

@tsa96
Copy link
Member

tsa96 commented Feb 23, 2024

Yeah providing a local container-only approach is reasonable, but wouldn't it be easier just dockerizing that entire process, rather than using devcontainers? I don't really know anything about devcontainers whatsoever, but keeping the number of dev technologies to a minimum is preferable, so I'd rather only offer devcontainers as an option if it can replace an existing requirement (e.g. Docker)

@alexwaibel
Copy link
Contributor Author

At their core, devcontainers are just a small editor config on top of the existing docker-compose stack that provides some metadata to your editor about which container in the compose stack to attach to, what plugins to install to the editor, what ports to forward, etc.

I opted to forward ports for example to avoid extra changes in this PR. Forwarded ports use an editor feature to make traffic appear to the container as if it came from localhost. We could instead modify the API process to listen on 0.0.0.0 instead of localhost and add ports: 3000:3000 to the api service in docker-compose. Same with some of the commands in devcontainer.json for installing node modules and starting the service, we could instead author a Dockerfile that includes these directly and that container. I do think that could be worth exploring, maybe in a follow-up PR, but for now with this PR I've done my best to not modify the way the frontend and backend services behave at all since I'm new to the project and didn't want to disrupt any existing workflows.

@tsa96
Copy link
Member

tsa96 commented Feb 24, 2024

Okay, that makes a lot of sense. And sorry, I've been so busy with other stuff lately I didn't sit down and look at devcontainers at all until just now. Glad that it's an open spec, and like you say they're just configurations for editors to follow, which is certainly helpful as a way to avoid getting hands dirty w Docker directly, having to choose what containers to attach to via CLI etc.

@tsa96
Copy link
Member

tsa96 commented Mar 19, 2024

Heads up that we don't accept merge commits, please rebase!

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.

2 participants