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

feat: backup db content locally and copy sql file to s3 bucket #271

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

blenzi
Copy link
Contributor

@blenzi blenzi commented Aug 2, 2023

This PR adds a script and adapts the code to be able to backup db content and upload it to s3 bucket

  • add scripts/dump_db.sh
  • s3.py S3Bucket.upload_file: support for ascii files
  • add python-dotenv dependency and load .env from root directory to be able to use env variables outside docker compose
  • docker-compose.yml: add volume db_backup to frontend service

- add scripts/dump_db.sh
- s3.py S3Bucket.upload_file: support for ascii files
- add python-dotenv dependency and load .env from root directory to be able to use env variables outside docker compose
- docker-compose.yml: add volume db_backup to frontend service
@blenzi blenzi requested a review from a team August 2, 2023 10:00
@ghost
Copy link

ghost commented Aug 2, 2023

Nice of you to open a PR 🙏
When you're ready and want to get it reviewed, post a comment in this Pull Request with this message: /quack review

@github-actions github-actions bot added topic: docs Improvements or additions to documentation topic: build Related to build, installation & CI topic: docker labels Aug 2, 2023
@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Merging #271 (e825116) into main (c10ff1c) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main      #271   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines           74        74           
=========================================
  Hits            74        74           
Flag Coverage Δ
client 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@blenzi
Copy link
Contributor Author

blenzi commented Aug 2, 2023

/quack review

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR 🙏

Other sections
  • [missing-all-definition] detected at src/app/config.py:L11
  • For a better experience, it's recommended to check the review comments in the tab Files Changed.


    Feedback is a gift! Quack will adjust your review style when you add reactions to a review comment

    src/app/config.py Show resolved Hide resolved
    fe51
    fe51 previously approved these changes Aug 2, 2023
    Copy link
    Member

    @fe51 fe51 left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Hi Bruno, thanks a lot for this PR that bring an interesting feature.

    I have made a small comment

    README.md Show resolved Hide resolved
    # Conflicts:
    #	poetry.lock
    #	src/app/config.py
    @github-actions github-actions bot added endpoint: media ext: tests and removed topic: build Related to build, installation & CI labels Aug 2, 2023
    @frgfm
    Copy link
    Member

    frgfm commented Oct 18, 2023

    Great idea, we definitely need some backup!
    One suggestion though: to avoid mixing DB backup & images in the same bucket, it might be worth setting a simple cron job instead?

    What I have in mind would be the CRON job doing in order:

    • dumping the sql DB from outside the postgres docker into a data/binded volume
    • using the backend docker to upload the dump from the volume (similar to the design here)
      That way we don't need to add any dependencies on root (otherwise, if we're OK with that, we can do everything without interacting with the backend container).

    What do you think?

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    endpoint: media ext: tests topic: docker topic: docs Improvements or additions to documentation
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants