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

Make the sandbox Python runtime completely transparent #2796

Merged
merged 11 commits into from
Jul 6, 2024

Conversation

Shimada666
Copy link
Contributor

@Shimada666 Shimada666 commented Jul 4, 2024

What is the problem that this fixes or functionality that this introduces? Does it fix any open issues?
Right now,python points to /opendevin/miniforge3/bin/python, which is not suitable. Users should use a clean Python interpreter that is separate from the python used by the opendevin runtime.

Give a brief summary of what the PR does, explaining any non-trivial design decisions
We should make the OpenDevIn Python runtime completely transparent to users. Users should continue using whatever interpreter python points to, like /usr/bin/python, instead of /opendevin/miniforge3/bin/python.

Other references

@tobitege tobitege requested a review from iFurySt July 4, 2024 16:41
@tobitege tobitege requested a review from xingyaoww July 4, 2024 17:36
Copy link
Collaborator

@xingyaoww xingyaoww left a comment

Choose a reason for hiding this comment

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

LGTM!

@xingyaoww xingyaoww enabled auto-merge (squash) July 4, 2024 18:01
@tobitege tobitege disabled auto-merge July 5, 2024 14:43
@tobitege
Copy link
Collaborator

tobitege commented Jul 5, 2024

@Shimada666 error in CI here:
https://github.com/OpenDevin/OpenDevin/actions/runs/9801641111/job/27065607085#step:7:523

FAILED tests/unit/test_sandbox.py::test_agnostic_sandbox_jupyter_agentskills_fileop_pwd - docker.errors.BuildError: The command '/bin/sh -c echo "" > /opendevin/bash.bashrcRUN { test -d /opendevin/miniforge3 && echo "/opendevin/miniforge3 already in base image"; } || { wget "https://github.com/conda-forge/miniforge/releases/latest/download/Miniforge3-$(uname)-$(uname -m).sh" && bash Miniforge3-$(uname)-$(uname -m).sh -b -p /opendevin/miniforge3 && bash -c ". /opendevin/miniforge3/etc/profile.d/conda.sh && conda config --set changeps1 False && conda config --append channels conda-forge" &&' returned a non-zero code: 2

I'll let this rerun and see if it still comes back this way.

@tobitege
Copy link
Collaborator

tobitege commented Jul 5, 2024

Repeated run shows the same error, just fyi.

@Shimada666
Copy link
Contributor Author

@tobitege
Thank you! I have noticed that test_ipython_module succeeded under codeactagent but failed under codeactsweagent. I don't have much expertise about codeactsweagent, so I am asking @xingyaoww to help me solve this issue. It should be working fine now!

@Shimada666
Copy link
Contributor Author

@xingyaoww @tobitege
Hi, I just fixed the errors in the agnostic image build script.. Could you help me rerun the CI tests? Thanks!

@tobitege
Copy link
Collaborator

tobitege commented Jul 6, 2024

@xingyaoww @tobitege Hi, I just fixed the errors in the agnostic image build script.. Could you help me rerun the CI tests? Thanks!

It's rolling now 😂

@tobitege tobitege enabled auto-merge (squash) July 6, 2024 10:23
@tobitege tobitege disabled auto-merge July 6, 2024 10:24
@iFurySt
Copy link
Collaborator

iFurySt commented Jul 6, 2024

everything looks good, is it ready to merge?

@Shimada666
Copy link
Contributor Author

@iFurySt Yes. please merge this PR, thanks!

@tobitege tobitege merged commit d22ff73 into All-Hands-AI:main Jul 6, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants