-
Notifications
You must be signed in to change notification settings - Fork 15
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: run-image for buildpacks with vscodium and python #485
Conversation
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.
Thanks @leafty! Just a couple minor comments
id: meta | ||
uses: docker/metadata-action@v5 | ||
with: | ||
images: ${{ env.DOCKER_PREFIX }}-buildpack-vscode-python |
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.
this is not technically a buildpack. We should probably call it something like <...>-vscode-python-runimage
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.
Yes, it's not a buildpacks, but it has very limited use outside of being used as a run image from buildpack builds.
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.
agreed, but calling it a "buildpack" carries a specific meaning, which is not accurate in this case so we should rename it.
@@ -46,6 +46,7 @@ jobs: | |||
- docker/vscode/conda.Dockerfile | |||
- docker/vscode/poetry.Dockerfile | |||
- docker/vscode/R.Dockerfile | |||
- docker/vscodium-buildpacks/base.Dockerfile |
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.
should we call this vscodium-runimage
? Or vscodium-buildpacks-runimage
? It's not a buildpack
DEFAULT_VSCODIUM_EXTENSIONS="\ | ||
ms-python.python \ | ||
ms-toolsai.jupyter \ | ||
" |
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.
This should be done in the docker image build, there's no reason to do it on launch
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.
Why not?
By doing this at launch:
- Any user can customize the extensions they want to have, just by tweaking the env vars.
- You don't need to re-build to use more extensions.
The install time is very fast, so it doesn't seem like it would annoy users. This is similar to how devcontainers work.
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.
Yes but those two will always be installed. So just put them in the image. And allow the user to specify additional ones if necessary.
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.
No, they will not alway be installed. If you specify VSCODIUM_EXTENSIONS=" "
then no extension is installed, if you specify VSCODIUM_EXTENSIONS="golang.go"
then you only have the go extension. The only issue at the moment is that custom env vars are not supported for Renku 2.0 sessions, so maybe this can be improved later to support command line arguments too.
This PR adds a new Docker image to be used as a run image for buildpacks with python and vscodium.