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 all db #133

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

feat: backup all db #133

wants to merge 5 commits into from

Conversation

nubpro
Copy link

@nubpro nubpro commented Sep 10, 2021

closes #109

Instead of only backing up the primary database (which must be the same name as the mysql instance), we are now backing up all databases, including the ones manually created by hand. We are however excluding the internal dbs used by mysql itself.

With this PR, I can now create additional databases on a single mysql container, saving us compute resources and $.

@josegonzalez
Copy link
Member

josegonzalez commented Sep 10, 2021

This breaks the :import command so I won't take it as is.

I'm also concerned about all other datastore plugins, and the fact that we don't have native, single-datastore multi-db support. I'd want to resolve that before implementing this (or concurrently I guess).

@nubpro
Copy link
Author

nubpro commented Sep 10, 2021

This breaks the :import command so I won't take it as is.

I'm also concerned about all other datastore plugins, and the fact that we don't have native, single-datastore multi-db support. I'd want to resolve that before implementing this (or concurrently I guess).

I did some basic testing on the import command.
I'm happy to report back that importing a sql file with multiple dbs will not break that particular command.
The expectation is that it will simply import the additional databases.

Here's my testing process:

  1. create container, db1 is created
  2. create db2 manually
  3. mysql:export db1 > backup.sql
  4. drop all tables from db1 and delete db2
    5.mysql:import db1 < backup.sql

Result:
db1 tables are restored. db2 is created and restored.

I understand that you are trying to ensure DX consistency across the rest of the datastores plugins, however for now, this will unblock my workflow and I'll continue with this

Let me know if there's anything else I would potentially break


[[ -n $SSH_TTY ]] && stty -opost
docker exec "$SERVICE_NAME" bash -c "printf '[client]\ndefault-character-set=utf8mb4\npassword=$PASSWORD\n' > /root/credentials.cnf"
Copy link
Member

Choose a reason for hiding this comment

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

Any reason we dropped the character set here?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, I probably left it out by accident

Comment on lines -138 to +141
docker exec -i "$SERVICE_NAME" mysql --user=root --password="$ROOTPASSWORD" "$DATABASE_NAME"

docker exec "$SERVICE_NAME" bash -c "printf '[client]\nuser=root\npassword=$ROOTPASSWORD\n' > /root/credentials.cnf"
docker exec -i "$SERVICE_NAME" mysql --defaults-extra-file=/root/credentials.cnf "$DATABASE_NAME"
docker exec "$SERVICE_NAME" rm /root/credentials.cnf
Copy link
Member

Choose a reason for hiding this comment

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

Why this particular change?

Copy link
Author

Choose a reason for hiding this comment

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

This change is introduced to surpress warnings from mysql

Warning: Using a password on the command line interface can be insecure.

https://stackoverflow.com/questions/20751352/suppress-warning-messages-using-mysql-from-within-terminal-but-password-written

@josegonzalez
Copy link
Member

Apologies for the late review, but I left some comments asking for clarification. Thanks for testing otherwise!

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.

Backup all databases instead of the one which is called like the service
2 participants