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

Chore/improv #1

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Chore/improv #1

wants to merge 3 commits into from

Conversation

naggingant
Copy link
Member

Needs your confirmation

  • Write files in /tmp instead of /backups which requires to be run as root, which is not very good for security
  • Set shell options so
    • undefined variables will be a failure
    • the program exits on the first failed line

@PuKoren
Copy link
Member

PuKoren commented May 19, 2022

Thank you! I think it does not cause any problem to change the location
The reason I initially did a separate directory was in case we want to mount a disk at this specific location (if the backup becomes too big to be stored in the container). So large databases could be backed up even if the host nodes have limited disk space

It might still work well and be possible to mount a disk on /tmp as I don't think the container do so much i/o on it. Another option could be to give rights to all users on /backups so all users can write in it

Just a reminder if we were to apply this, the VOLUME in the Dockerfile will have to be updated or removed, and the README will need to be updated

@naggingant naggingant marked this pull request as draft June 13, 2022 00:31
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