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(sandbox): mount_dir for ssh_box #1333

Merged
merged 2 commits into from
Apr 24, 2024

Conversation

xingyaoww
Copy link
Collaborator

@xingyaoww xingyaoww commented Apr 24, 2024

The default for WORKSPACE_MOUNT_PATH is None, which causes issues when try to map it into SSHBox.
Right now, SSHBox first read for WORKSPACE_MOUNT_PATH then WORKSPACE_BASE.

Based on our README.md, it seems to me that both of these variable means the same thing. Can we merge them together into one variable (i.e., we keep WORKSPACE_MOUNT_PATH and remove WORKSPACE_BASE)?

...
    -e WORKSPACE_MOUNT_PATH=$WORKSPACE_BASE \
    -v $WORKSPACE_BASE:/opt/workspace_base \
...

@rbren
Copy link
Collaborator

rbren commented Apr 24, 2024

I think it might be good to move this logic to config.py--if WORKSPACE_MOUNT_PATH is unset, set it to WORKSPACE_BASE

I believe the logic you have here only works if running outside docker--inside docker it would probably break (though both variables should be set in that case)

@xingyaoww
Copy link
Collaborator Author

xingyaoww commented Apr 24, 2024

Good point! I have moved the code to config.py!

@rbren rbren merged commit e6ecbd5 into OpenDevin:main Apr 24, 2024
5 checks passed
@rbren
Copy link
Collaborator

rbren commented Apr 24, 2024

Thanks!

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.

None yet

2 participants