-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Feature: Added devcontainer support #1252
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
Open
PraaneshSelvaraj
wants to merge
9
commits into
PokeAPI:master
Choose a base branch
from
PraaneshSelvaraj:feature/devcontainer
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
700aa71
feat: devcontainer
PraaneshSelvaraj d06da56
Merge branch 'PokeAPI:master' into feature/devcontainer
PraaneshSelvaraj efd6da1
fixed devcontainer
PraaneshSelvaraj aba472c
Added devcontainer commands
PraaneshSelvaraj 42672ee
devcontainer instructions
PraaneshSelvaraj 1bf02bd
Merge branch 'master' into feature/devcontainer
PraaneshSelvaraj ae80aa6
devcontainer dockerfile
PraaneshSelvaraj 3b1821a
Merge branch 'master' into feature/devcontainer
PraaneshSelvaraj ea839e3
Merge branch 'master' into feature/devcontainer
PraaneshSelvaraj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
ARG PYTHON_VERSION=3.12-bullseye | ||
FROM mcr.microsoft.com/vscode/devcontainers/python:${PYTHON_VERSION} | ||
|
||
ENV PYTHONUNBUFFERED=1 | ||
ENV DJANGO_SETTINGS_MODULE='config.docker-compose' | ||
|
||
RUN mkdir /code | ||
WORKDIR /code | ||
|
||
RUN apt-get update && \ | ||
apt-get install -y --no-install-recommends \ | ||
libpq-dev \ | ||
build-essential \ | ||
rustc \ | ||
cargo && \ | ||
apt-get clean && rm -rf /var/lib/apt/lists/* | ||
|
||
RUN addgroup --gid 1001 --system pokeapi && \ | ||
adduser --uid 1001 --system --ingroup pokeapi --shell /bin/bash pokeapi | ||
|
||
RUN chown -R pokeapi:pokeapi /code | ||
|
||
USER pokeapi | ||
|
||
ADD requirements.txt /code/ | ||
ADD test-requirements.txt /code/ | ||
RUN python3 -m pip install -r test-requirements.txt --no-cache-dir | ||
|
||
ADD . /code/ | ||
|
||
CMD ["sleep", "infinity"] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
{ | ||
"name": "PokeAPI", | ||
|
||
// Use multiple docker-compose files for development and overrides | ||
"dockerComposeFile": [ | ||
"../docker-compose.yml", | ||
"./docker-compose.override.yml" | ||
], | ||
|
||
// Target container service to attach VS Code workspace | ||
"service": "app", | ||
|
||
// Folder inside container where workspace is mounted | ||
"workspaceFolder": "/code", | ||
|
||
"customizations": { | ||
"vscode": { | ||
"settings": { | ||
"python.defaultInterpreterPath": "/usr/local/bin/python", | ||
|
||
// Use bash as the default shell in the integrated terminal | ||
"terminal.integrated.profiles.linux": { | ||
"bash": { | ||
"path": "/bin/bash" | ||
} | ||
}, | ||
"terminal.integrated.defaultProfile.linux": "bash", | ||
|
||
// Enable automatic formatting on save with Black for Python files | ||
"[python]": { | ||
"editor.defaultFormatter": "ms-python.black-formatter", | ||
"editor.formatOnSave": true | ||
} | ||
}, | ||
|
||
// Extensions to install automatically in the container's VS Code instance | ||
"extensions": [ | ||
"ms-python.python", | ||
"ms-python.black-formatter" | ||
] | ||
} | ||
}, | ||
|
||
"features": { | ||
"ghcr.io/devcontainers/features/common-utils:2": { | ||
"installZsh": "false", | ||
"installOhMyZsh": "false", | ||
"configureZshAsDefaultShell": "false" | ||
}, | ||
"ghcr.io/devcontainers/features/git:1": {}, | ||
"ghcr.io/devcontainers/features/docker-outside-of-docker": {} | ||
}, | ||
|
||
// Post-create commands: | ||
"postCreateCommand": "git config --global core.autocrlf input && git config --global --add safe.directory /code", | ||
|
||
"remoteUser": "pokeapi", | ||
|
||
// Ports to forward from container to host for easy access | ||
"forwardPorts": [80, 443, 8000, 8080], | ||
|
||
// Custom port labels and auto-forward notification settings | ||
"portsAttributes": { | ||
"8000": { | ||
"label": "App", | ||
"onAutoForward": "notify" | ||
}, | ||
"80": { | ||
"label": "Web HTTP", | ||
"onAutoForward": "notify" | ||
}, | ||
"443": { | ||
"label": "Web HTTPS", | ||
"onAutoForward": "notify" | ||
}, | ||
"8080": { | ||
"label": "GraphQL", | ||
"onAutoForward": "notify" | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
version: '2.4' | ||
|
||
services: | ||
app: | ||
build: | ||
context: . | ||
dockerfile: ./.devcontainer/Dockerfile | ||
user: pokeapi |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hi, Can't we use this override https://github.com/PokeAPI/pokeapi/blob/master/docker-compose.override.yml ?
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.
The image from Docker Hub automatically starts the Gunicorn server, but for our Dev Container setup, we actually don't want it running by default. I've overridden the CMD to prevent that, as we use
make
commands to handle server startup.To keep the main
docker-compose.override.yml
clean for its original purpose, I set up a separate override file just for the Dev Container here:.devcontainer/docker-compose-override.yml
.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.
For Dev Container features like docker-outside-of-docker (and others), we typically need a Debian-based image for them to work properly.
Since our current image is built on Alpine Linux, I would suggest creating a separate
Dockerfile
inside the .devcontainers folder. This dedicatedDockerfile
would then build our Dev Container image using a Debian base.What do you think of this?
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.
Hi, honestly I think it's too much work with few benefits :) If you like you could work on other issues.
Top two would be: helping @Indyandie with the new Astro website, polishing the openapi schema and make it available in the current website
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.
Hey @Naramsim,
Totally get where you're coming from on keeping things simple, and I completely agree that we shouldn't add complexity without good reason.
But, just wanted to quickly mention why I think Dev Containers could actually save us headaches long-term: it means anyone can jump in and contribute instantly, without wrestling with setup. It ensures everyone's development environment is the same, so we avoid "it works on my machine" issues. Plus, it works well with features like cloud development (like GitHub Codespaces).
It feels like a small upfront effort for a much smoother ride for all contributors, especially new ones.
If you still think this isn't required, I'll totally drop it and focus on other things.
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.
Hi @PraaneshSelvaraj sorry for coming back only now. I'm currently unable to fully review the devcontainer file honestly. Maybe I'll just merge it in and see if issues arise when people start using it. Still unsure. I have limited time currently
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.
Hey @Naramsim , Sounds good.