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 docker usage readme #2406

Merged
merged 2 commits into from
Apr 21, 2023
Merged

Fix docker usage readme #2406

merged 2 commits into from
Apr 21, 2023

Conversation

pkqs90
Copy link
Contributor

@pkqs90 pkqs90 commented Apr 18, 2023

Background

Since Dockerfile uses /home/appuser as workdir, the mounted volume should also be /home/appuser, or else we can't persist the written files.

https://github.com/Significant-Gravitas/Auto-GPT/blob/master/Dockerfile#L25

Changes

Documentation

Test Plan

Tested locally.

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes

KKoperski
KKoperski previously approved these changes Apr 18, 2023
nponeccop
nponeccop previously approved these changes Apr 19, 2023
@nponeccop nponeccop added B8 docker documentation Improvements or additions to documentation labels Apr 19, 2023
hdkiller
hdkiller previously approved these changes Apr 20, 2023
@hdkiller
Copy link
Contributor

The docker image in latest master in deed uses that workdir.

=> CACHED [ 7/11] WORKDIR /home/appuser                                                                                                                                                        ```

@np
Copy link

np commented Apr 20, 2023

LGTM.
There are two more occurrences of /app in docker-compose.yml.

@pkqs90 pkqs90 dismissed stale reviews from hdkiller, nponeccop, and KKoperski via 8364426 April 21, 2023 06:18
@pkqs90
Copy link
Contributor Author

pkqs90 commented Apr 21, 2023

LGTM. There are two more occurrences of /app in docker-compose.yml.

Updated

@richbeales richbeales merged commit e20d388 into Significant-Gravitas:master Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker documentation Improvements or additions to documentation size/xs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants