-
Notifications
You must be signed in to change notification settings - Fork 44.4k
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
Improve docker setup & config #1843
Improve docker setup & config #1843
Conversation
702dde0
to
6a82d01
Compare
6a82d01
to
2ade317
Compare
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
May be worth adding https://hub.docker.com/r/selenium/standalone-chrome/ See here for examples |
@ntindle currently, Chrome and Firefox along with some other tools are installed in the docker container, see Dockerfile. I'm not convinced this is the way, but also don't have time to dive into it further atm. Do you think it is worth it to use standalone selenium containers instead of running headless browsers in the |
IMO using the selenium headless versions with their drivers makes a lot of sense. Otherwise we’d basically need to duplicate the work they’ve done and will continue to do when things change |
Could you make a PR to implement that as soon as this one merges? Would be nice :) |
Can do |
@mikekelly care to review? |
Quite a bit of this is covering ground from #1199 and will conflict with it |
This combined with a smoke test of the docker container in CI would be top notch |
Background
The current Dockerfile and docker-compose file seem to have unnecessary convolution of configuration and don't contain an optimal configuration out of the box. Also, it is not consistent with the README. This PR attempts to fix that.
This PR either conflicts with or deprecates PRs #1830 and #1199
Changes
apt-get
actions into one layer/home/appuser
appuser
with UID 1000 to match probable permissions on Linux host/app
(as documented in the README) which is a widely used standard, making it easier to work with for new usersdocker-compose
config/app
./logs/
,./auto_gpt_workspace/
and./ai_settings.yaml
in/app
Documentation
New configuration is consistent with current configuration; no update needed.
Test Plan
No Python code is changed, so testing succeeds if the container builds and runs properly after these changes, which it does.
PR Quality Checklist