Updated README.md #46
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Comment on lines
+70
to
+72
| `docker exec db-auto-backup SCHEDULE=` | ||
|
|
||
| `docker exec ./db-auto-backup.py` |
There was a problem hiding this comment.
Issue: I'm not sure these commands will actually work, the new SCHEDULE value won't persist. You probably want:
docker-compose exec backup SCHEDULE= ./db-auto-backup.py
| @@ -13,30 +13,60 @@ A script to automatically back up all databases running under docker on a host | |||
|
|
|||
| ## Installation | |||
|
|
|||
| This container requires access to the docker socket. This can be done either by mounting `/var/lib/docker.sock`, or using a HTTP proxy to provide it through `$DOCKER_HOST`. | |||
| This container requires access to the docker socket. This should always be done, for security reasons, using a [HTTP proxy](https://github.com/Tecnativa/docker-socket-proxy) which this container can then access through the `DOCKER_HOST` environment variable. | |||
There was a problem hiding this comment.
There's a lot of wording change here, which doesn't seem to add much. I'm ok with talking about the socket before using a proxy (even if it is worse).
I personally always use $ as a prefix for environment variables, although I agree that might not be clear. I'd be ok with a simpler patch which just rewords around that instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed, some proposed changes to the README that would have saved me a fair bit of time! Feel free to cherry pick any parts you like!