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

Install Multiple Python Versions in the Image #181

Merged
merged 7 commits into from
Aug 29, 2023

Conversation

MarkKoz
Copy link
Member

@MarkKoz MarkKoz commented Aug 20, 2023

#158
This is an alternative implementation to #175

This just focuses on getting multiple Python interpreter versions into the image. There will be a follow-up PR with the API changes. The goal for the API is for users to be able to pass an arbitrary path to an executable binary (e.g. a path to a different Python interpreter).

See the changes in .github/CONTRIBUTING.md for an overview of how multiple Python versions are managed.

du -h --max-depth=1 /lang/python
93M	/lang/python/3.11
88M	/lang/python/3.12
181M	/lang/python
REPOSITORY                            TAG         IMAGE ID       CREATED              SIZE
ghcr.io/python-discord/snekbox        latest      77b91b95f4da   About a minute ago   511MB
ghcr.io/python-discord/snekbox-venv   dev         7fc66456bbdd   26 minutes ago       600MB

I observed smaller sizes (~70 MB) on Debian 12 (bookworm), but I've had trouble with test failures on bookworm, so the image remains on Debian 11 (buster) for now. About a third of the size seems to be occupied by the shared library.

du -h /lang/python/3.11/lib/libpython3.11.so.1.0
32M	/lang/python/3.11/lib/libpython3.11.so.1.0

If anyone knows of additional compiler flags or configuration options to bring down the size, please share!

Each Python interpreter build takes around 6 minutes, and that's with full optimisation enabled. Thankfully each build stage can run in parallel.

It's using a stable version of Debian, so it's redundant to lock
packages to specific versions.
Get some of the NsJail build dependencies pre-installed thanks to the
base image.
Separate snekbox's Python interpreter from the interpreter used by
NsJail. This allows for the interpreters to be updated on different
cadences and provides better isolation of packages.

Each Python interpreter adds about 70 MB to the built image.
Re-use already built COPY layers in subsequent builds even if the
previous layers have changed, which is especially helpful when copying
from another build stage.

See https://docs.docker.com/engine/reference/builder/#copy---link
Prevent an empty exec_bin.args from manifesting as an empty string in
the fully built arguments.
@MarkKoz MarkKoz added type: feature New feature or request area: dependencies Related to package dependencies and management area: API Related to or causes API changes labels Aug 20, 2023
@MarkKoz MarkKoz marked this pull request as draft August 20, 2023 03:44
@MarkKoz MarkKoz changed the title Feat/158/multi version Support Multiple Python Versions Aug 20, 2023
Need to use `export` to set vars when && is used between the commands.
@coveralls
Copy link

Coverage Status

coverage: 90.845%. remained the same when pulling 0b0b0fd on feat/158/multi-version into 3888578 on main.

@ChrisLovering
Copy link
Member

FWIW, I think this is a good approach as far as the image is concerned.

@MarkKoz MarkKoz changed the title Support Multiple Python Versions Install Multiple Python Versions in the Image Aug 26, 2023
@MarkKoz MarkKoz marked this pull request as ready for review August 26, 2023 19:25
deployment.yaml Outdated Show resolved Hide resolved
jb3
jb3 previously requested changes Aug 29, 2023
Copy link
Member

@jb3 jb3 left a comment

Choose a reason for hiding this comment

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

all looks good and works locally -- just needs the --user change for deps and should be good to go!

deployment.yaml Show resolved Hide resolved
Due to the way that `find` executes -exec arguments we need to
run the pip install's inside another `sh` instance so that the
PYTHONUSERBASE environment variable is correctly picked up.

Additionally, we need to specify `--user` so that pip respects
the PYTHONUSERBASE variable at all.
@jb3 jb3 dismissed their stale review August 29, 2023 18:02

Dismissing my review since I've added a commit to resolve comments

@jb3 jb3 enabled auto-merge August 29, 2023 18:05
@ChrisLovering ChrisLovering merged commit f85116a into main Aug 29, 2023
6 checks passed
@ChrisLovering ChrisLovering deleted the feat/158/multi-version branch August 29, 2023 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Related to or causes API changes area: dependencies Related to package dependencies and management type: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants