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.

Sign up for GitHub

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

Jump to bottom

OPIK-665: Add Python code executor PoC Docker sandbox #1058

Merged
merged 3 commits into from
Jan 17, 2025
Merged

OPIK-665: Add Python code executor PoC Docker sandbox #1058

merged 3 commits into from
Jan 17, 2025

Conversation

andrescrz
Copy link
Collaborator

@andrescrz andrescrz commented Jan 16, 2025 •
edited
Loading

Details

Moved the metric scoring functionality to a subprocess containerised in Docker, using an image with a minimal Python environment with opik package installed.

Minor updates in the endpoint signature to accommodate for future executors of other languages e.g: TypeScript/JavaScript.

Cleaned up some no longer necessary code. Reviewed all dependencies, normal and test, and removed unnecessary ones.

In preparation for the deployment, added a Dockerfile and a gunicorn server.

Issues

OPIK-665

Testing

  • Passed the whole test suite.
  • Tested manually both with a local Flask server or with a containerised gunicorn server.
  • Added integration tests with more error cases.

Documentation

@andrescrz andrescrz force-pushed the andrescrz/OPIK-665-add-python-code-executor-poc-docker-sandbox branch from 1a152d1 to 4071b8c Compare January 16, 2025 09:26
@andrescrz andrescrz self-assigned this Jan 16, 2025
@andrescrz andrescrz added the enhancement New feature or request label Jan 16, 2025
@andrescrz andrescrz marked this pull request as ready for review January 16, 2025 09:27
@andrescrz andrescrz requested review from a team as code owners January 16, 2025 09:27
liyaka
liyaka previously requested changes Jan 16, 2025
View reviewed changes
apps/opik-python-backend/Dockerfile Outdated Show resolved Hide resolved
@andrescrz andrescrz force-pushed the andrescrz/OPIK-665-add-python-code-executor-poc-docker-sandbox branch from 181946c to 631e0c2 Compare January 16, 2025 10:51
@andrescrz andrescrz requested a review from a team January 16, 2025 11:54
alexkuzmik
alexkuzmik reviewed Jan 17, 2025
View reviewed changes
Copy link
Collaborator

@alexkuzmik alexkuzmik left a comment

Choose a reason for hiding this comment

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

Looks good to me!
I only left a couple of comments for improving error experience.

apps/opik-python-backend/src/opik_backend/scoring_commands.py Outdated Show resolved Hide resolved
apps/opik-python-backend/src/opik_backend/docker_runner.py Outdated Show resolved Hide resolved
@andrescrz andrescrz force-pushed the andrescrz/OPIK-665-add-python-code-executor-poc-docker-sandbox branch from 631e0c2 to 31c3869 Compare January 17, 2025 16:15
@andrescrz andrescrz dismissed liyaka’s stale review January 17, 2025 16:20

Addressed comments.

@andrescrz andrescrz merged commit 688be1b into main Jan 17, 2025
@andrescrz andrescrz deleted the andrescrz/OPIK-665-add-python-code-executor-poc-docker-sandbox branch January 17, 2025 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexkuzmik alexkuzmik alexkuzmik approved these changes

@liyaka liyaka Awaiting requested review from liyaka

Assignees

@andrescrz andrescrz

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andrescrz @liyaka @alexkuzmik