DevContainer! #11443
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| // For format details, see https://aka.ms/devcontainer.json. | ||
| { | ||
| "name": "Kolibri Dev Container", | ||
| "build": { | ||
| "dockerfile": "../docker/base.dockerfile", | ||
| "context": ".." | ||
| }, | ||
| "features": { | ||
| "ghcr.io/devcontainers/features/common-utils:2": { | ||
| "username": "devuser", | ||
| "upgradePackages": true, | ||
| "installZsh": false, | ||
| "configureZshAsDefaultShell": false, | ||
| "installOhMyZsh": false, | ||
| "installOhMyZshConfig": false, | ||
| "nonFreePackages": false | ||
| } | ||
| }, | ||
| "workspaceMount": "source=${localWorkspaceFolder},target=/kolibri,type=bind", | ||
| "workspaceFolder": "/kolibri", | ||
| "forwardPorts": [ | ||
| 3000 | ||
| ], | ||
| "portsAttributes": { | ||
| "3000": { | ||
| "label": "Webpack port", | ||
| "onAutoForward": "silent" | ||
| } | ||
| }, | ||
| "containerEnv": { | ||
| "KOLIBRI_RUN_MODE": "docker_dev", | ||
| "KOLIBRI_HOME": "/kolibri/.devcontainer/persist_dir/.kolibri" | ||
| }, | ||
| "remoteEnv": { | ||
| "GIT_EDITOR": "nano" | ||
| }, | ||
| "customizations": { | ||
| "vscode": { | ||
| "extensions": [ | ||
| "ms-python.python", | ||
| "ms-python.vscode-pylance", | ||
| "octref.vetur", | ||
| "editorconfig.editorconfig" | ||
| ] | ||
| } | ||
| }, | ||
| "onCreateCommand": "yarn install --non-interactive --frozen-lockfile && pip install -e . && pre-commit install", | ||
| "containerUser": "devuser", | ||
| "remoteUser": "devuser" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| This directory is used by the devcontainer to persist kolibri related directories. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,43 +1,30 @@ | ||
| # This Dockerfile serves as a base image for Kolibri installations. It installs python, required OS packages, node, npm, yarn and python dependencies. | ||
|
|
||
| # Declare python version argument. | ||
| ARG PYTHON_VARIANT="3.9-slim" | ||
| FROM python:${PYTHON_VARIANT} | ||
|
|
||
| # Declare node major version argument. | ||
| ARG NODE_MAJOR_VERSION="16" | ||
|
|
||
| # Install only the absolutely-required packages to run Kolibri -- Git, Git-LFS, ip, ps and Node (plus npm). | ||
| RUN DEBIAN_FRONTEND=noninteractive apt-get update && apt-get install -y ca-certificates curl gnupg git git-lfs procps iproute2 \ | ||
| && mkdir -p /etc/apt/keyrings \ | ||
| && curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key | gpg --dearmor -o /etc/apt/keyrings/nodesource.gpg \ | ||
| && echo "deb [signed-by=/etc/apt/keyrings/nodesource.gpg] https://deb.nodesource.com/node_$NODE_MAJOR_VERSION.x nodistro main" | tee /etc/apt/sources.list.d/nodesource.list \ | ||
| && apt-get update && apt-get install -t nodistro -y nodejs | ||
|
|
||
| # Install yarn. | ||
| RUN npm install --global yarn | ||
|
|
||
| # Change working directory for the commands that follow. | ||
| WORKDIR /kolibri | ||
|
There was a problem hiding this comment. Presumably, one could have built an image of Kolibri, containing its source, with this file before this change. After this change, which removes The There was a problem hiding this comment. Initially, my thought process was on those lines so I kept the devcontainer dockerfile completely isolated from deployable dockerfiles. But then eventually it turned out that keeping maintainability in mind utilising the base dockerfile would be better so went that route. There was a problem hiding this comment.
These changes are backwards incompatible though, so you haven't kept any maintainability. The two images, a dev container image and a deployable image, have some commonalities such as installing python requirements, but overall have different use cases and responsibilities. For instance, a deployable Kolibri image doesn't need node.js (an existing dilemna) because node.js is only needed for building static JS/CSS resources. There was a problem hiding this comment. Yeah, so having a separate dockerfile for devcontainer is better, isn't it? 🤔 There was a problem hiding this comment. We discussed this a while back, and I asked that you think about this holistically considering we have the desire to make running Kolibri in a docker container more well documented and to potentially have a well-defined shippable image. As I said, the two use cases have commonalities in how define and build these docker images but they have different responsibilities. With There was a problem hiding this comment. I totally agree with you on this that the current approach of this PR has put everything in just the I think we need to decide on how our shippable docker image for Kolibri will look like so that we can decide whether we should build our devcontainer image on top of that or not. I think I should talk to David to sort this out as soon as we can...? |
||
|
|
||
| # Copy python dependency files and install them. | ||
| COPY requirements/ ./requirements | ||
| COPY requirements.txt ./ | ||
| RUN pip install --upgrade --root-user-action=ignore --default-timeout=150 pip \ | ||
| && pip install -r requirements.txt --upgrade --root-user-action=ignore --default-timeout=150 \ | ||
| && pip install -r requirements/dev.txt --upgrade --root-user-action=ignore --default-timeout=150 \ | ||
| && pip install -r requirements/test.txt --upgrade --root-user-action=ignore --default-timeout=150 \ | ||
| && pip install -r requirements/docs.txt --upgrade --root-user-action=ignore --default-timeout=150 | ||
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.
There isn't much benefit to setting up nodesource as a apt-repo inside the image. Also, there are better ways to handle this in a docker file, to better take advantage of caching.
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.
Nodesource suggested setting up apt-repo so that
apt updateandapt installcan figure out nodesource repo.https://github.com/nodesource/distributions?tab=readme-ov-file#installation-instructions
Regarding caching, I tried to keep the commands in one RUN command to make best use of caching. If there's more we can do to further improve on that then please let me know and I will do it.
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.
Right, they'd suggest an apt-repo, but this is a container! It will likely only ever be installed once so it'll never use it for checking for updates!
For example, if the node.js version changes, this installs
ca-certificates curl gnupg git git-lfs procps iproute2all over again. So no this isn't making the best use of caching.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.
So without apt-repo, how should we install node?
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.
Your changes here modified how we were doing it before, which was not to use the apt-repo.
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.
Yeah right, I will fix that with our refactor of base & dev dockerfiles.