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

fix: workspace folder permission & app container cannot access client API #3300

Merged
merged 24 commits into from
Aug 8, 2024

Conversation

xingyaoww
Copy link
Collaborator

@xingyaoww xingyaoww commented Aug 8, 2024

What is the problem that this fixes or functionality that this introduces? Does it fix any open issues?

Requires #3299 and #3301 to be merge first.

Issue 1: Workspace folder permission

In #3282, I tried to fix the dubious ownership issue by "chown" the whole mounted folder to the current user_id, but it raised a new issue while I was trying to fix things in #3299: When I mount a folder own by "me" with group "staff", that "chmod" will fail, possibly due to MacOS's permission management.

On second thought, as long as the mounted workspace is owned by the current user "me" and the user id is set properly to the OpenDevin application, this permission issue / dubious ownership should not happen in the first place. I think the original issue probably arises when i try have the agent running as root and commit to my workspace (owned by "me").

Issue 2: After build OpenDevin app docker application, the backend cannot access the runtime API

Because It will try to access localhost:{port}, instead of host.docker.internal:{port}.


Give a summary of what the PR does, explaining any non-trivial design decisions

Fix issue 1

  • Remove extra git config setting (these are already done in "client.py")
  • Only chown when /workspace does not exists (in the case of mounted workspace, we should assume /workspace is already there)
  • Change the test case to do not use mounted folder: pytest will create tmp dir owned by root, while the UID we send to OpenDevin is actually the current user "me", which cause weird behavior of the test.
  • Improve the logging, especially when we are waiting to connect to the API server inside a docker container. Before this change, it will hang for a long time without giving any log / errors.

Fix issue 2

  • Add "api_hostname" to config, and set it to "host.docker.internal" in app docker

Other references

@xingyaoww xingyaoww changed the title fix: workspace folder permission fix: workspace folder permission & app container cannot access client API Aug 8, 2024
Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

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

Clear enough, thanks!

@neubig neubig merged commit 2e6b08d into main Aug 8, 2024
2 checks passed
@neubig neubig deleted the xw/fix-git-ownership branch August 8, 2024 23:28
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.

2 participants