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

Allow Evaluation Using Multiple Python Versions #158

Open
HassanAbouelela opened this issue Nov 13, 2022 · 3 comments
Open

Allow Evaluation Using Multiple Python Versions #158

HassanAbouelela opened this issue Nov 13, 2022 · 3 comments
Labels
area: backend Related to internal functionality and utilities status: planning Discussing details type: feature New feature or request

Comments

@HassanAbouelela
Copy link
Member

Description

Install multiple python versions within the container, and add an option in the API to select between them.

Rationale

Currently, the bot supports evaluation of both 3.10 and 3.11 code, something I think should be kept and expanded in the future (to include major/important python versions). This is achieved by using the latest 3.10 image published here. This is fine for now, but it makes it harder to provide feature parity and avoid issues when users switch between modes, and it makes it difficult to patch and fix security issues.

Implementation

The way I see us doing this is by building all the versions of python we want to support directly into the image, then aiming the jail at the right executable. This would simplify implementation by having one server, with one codebase, while allowing us to evaluate user code on any version.

Specifics

Some more specific implementation details that can be up for debate:

  1. The docker image will simply install whatever versions of python you pass to it in the build arg, to well defined locations
  2. We'll build two images in CI, one with all versions we want supported, and one lightweight for dev
  3. We can expose an endpoint to get all supported versions
  4. We should investigate the best way to install all the versions to the docker image. I imagine the best solution is to just start from an image with all the required build-tools, install all the python versions, and use that as the base for the image. I found this docker image which has all the versions from 2.3-3.9 installed, and it comes out to a whooping... 600 MBs! I don't think this specific project would work for us since it hasn't been updated since 3.9, but it does make the idea seem feasiable.
  5. Are the exposed packages going to be an issue, or can we simply install all the same versions to all python interpreters.
@HassanAbouelela HassanAbouelela added type: feature New feature or request status: planning Discussing details area: backend Related to internal functionality and utilities labels Nov 13, 2022
@onerandomusername
Copy link
Contributor

Are the exposed packages going to be an issue, or can we simply install all the same versions to all python interpreters.

I think it would be a nice idea to be able to have different packages installed. Any c-extension could compile differently based on the targeted python version, so it would be nice to have different versions.

ChrisLovering added a commit to python-discord/bot that referenced this issue May 30, 2023
This is until snekbox supports multi-version natively
python-discord/snekbox#158
ChrisLovering added a commit to python-discord/kubernetes that referenced this issue May 30, 2023
A breaking change was made to snekbox, and we decided to not backport the change to a 3.10 container.

Instead we have opted to support multiple Python versions within snekbox natively.
python-discord/snekbox#158
ChrisLovering added a commit to python-discord/kubernetes that referenced this issue May 30, 2023
A breaking change was made to snekbox, and we decided to not backport the change to a 3.10 container.

Instead we have opted to support multiple Python versions within snekbox natively.
python-discord/snekbox#158
ChrisLovering added a commit to python-discord/site that referenced this issue May 30, 2023
With python-discord/bot#2618 there will only be 1 snekbox container, that runs the latest verison of snekbox.

Supporting multiple versions of snekbox will be covered by python-discord/snekbox#158 where a single instance of snekbox will nativly support multiple Python verisons.
ChrisLovering added a commit to python-discord/site that referenced this issue May 30, 2023
With python-discord/bot#2618 there will only be 1 snekbox container, that runs the latest verison of snekbox.

Supporting multiple versions of snekbox will be covered by python-discord/snekbox#158 where a single instance of snekbox will nativly support multiple Python verisons.
ChrisLovering added a commit to python-discord/site that referenced this issue May 30, 2023
With python-discord/bot#2618 there will only be 1 snekbox container, that runs the latest verison of snekbox.

Supporting multiple versions of snekbox will be covered by python-discord/snekbox#158 where a single instance of snekbox will nativly support multiple Python verisons.
ChrisLovering added a commit to python-discord/bot that referenced this issue May 30, 2023
* Temporarily remove suport for evaling under 3.10

This is until snekbox supports multi-version natively
python-discord/snekbox#158

* Remove special case 3.11 snekbox container

3.11 is the default in snekbox:latest now, so no need to have a special container. This also removes all notion of there being diferent containers for different snekbox versions
@ChrisLovering
Copy link
Member

ChrisLovering commented Oct 3, 2023

The snekbox Dockerfile now installs both 3.12.0 and 3.13-dev to /lang/python/<version>.
There is also /lang/python/default which is a symlink to /lang/python/3.12/ which is what snekbox currently uses for eval.

Now we need to update snekbox's API to support multiple versions.
The following is my opinion on how to do that.

A new endpoint GET /versions is added, which returns all versions snekbox supports. The response body would include the name, a human friendly name, and whether it is a default. this would allow clients to easily give their users a list of versions that they can use.

POST /eval will be updated to have an optional version key, which is the version to eval the code under. This value of this key would be the name field from GET /versions. If it is not given, then the version marked as default in GET /versions is used instead.

@MarkKoz
Copy link
Member

MarkKoz commented Oct 4, 2023

@ChrisLovering This was essentially the approach taken in #175, which has some discussion.

I think it is better to accept an arbitrary path to a binary to execute. I'm hesitant to make design decisions that pigeon-hole snekbox into just a Python evaluator or just a code evaluator in general. Technically there is not much difference between that and sandboxing arbitrary binaries.

On the other hand, that is the common use case, so we can compromise here. I don't see a strong need for exposing versions via an API, but if we were to do it, I think it should return the paths to the binaries, so they can be passed to the eval endpoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: backend Related to internal functionality and utilities status: planning Discussing details type: feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants