-
Notifications
You must be signed in to change notification settings - Fork 537
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
Use db_backup + management commands to implement data dump/load/seed #22693
Conversation
be0348b
to
15df10b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc @diox had concerns that django's dumpdata
wasn't going to work for us, but I don't know the details.
15df10b
to
d195abe
Compare
d195abe
to
9910061
Compare
I ran into these issues myself. I think it could be possible to mitigate but I found a library that does mysql level dumping/loading and also handles files.. so pretty much exactly what we are aiming for.. It's a dev dep and only installed on dev so I think it's relatively low risk to add. Also seems well maintained and I read through the code fairly easily. |
9910061
to
21fc275
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using django-dbbackup seems the way to go, yeah. dumpdata
and loaddata
are a pain to work with when you have complex relationships (especially the kind that we have).
189201c
to
748234a
Compare
f10455b
to
1dd6aa7
Compare
1dd6aa7
to
6689df2
Compare
Use db_backup to manage database dump/load TMP: fix docs Align file names and command names Reindex on data_load Fix broken class assignment and add better error handling TMP: Why is this test failing ..now Remove dead settings
@KevinMind I'm attempting to follow the testing instructions but I don't think they're up to date? |
@eviljeff updated. You should run |
6689df2
to
a04dba3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r+wc - for me, just need to apply the suggestion to reintroduce create_db
(or a similar change)
Use db_backup to manage database dump/load TMP: fix docs Align file names and command names Reindex on data_load Fix broken class assignment and add better error handling TMP: Why is this test failing ..now Remove dead settings
Relates to: mozilla/addons#15020
Unblocks: #22663
Description
Context
In order to include data initialization into the make up flow, we need a better way to dump/load/seed data. This PR moves the mechanism for exporting/importing data to use the db-backup lib orchestrated by management commands, includes test coverage for these behaviours and removes the concept from our make files.
db-backup is a light wrapper around django database connectors to use mysql commands for dumping/loading. This side steps issues we have with using native django dump/load data commands. Before this patch, we had a db connector and could implement this logic ourselves.. but looking at this lib it is well maintained, has a lot of activity and is very transparent looking through the source code. I cannot imagine implementing something much different then what they already have.
This unblocks the initialization part because the command for initializing data in make up will use seeding (which uses dumping) and loading in order to make sure the database is in the "correct" state based on the the arguments passed by the developer.
This can be extracted as a separate PR to make review and testing easier as this part is relatively low impact.
From this PR we also get synchronization of the ./storage directory with the database backup which squashes a whole range of bugs related to mismatches from db file references and the underlying file system.
Testing
Expect a directory with the
timestamp
name is added to ./backups. It should include a filedb.sql
and a storage filestorage.tar
which is an exact replica of the ./storage directory.Optionally pass an argument
ARGS=--name foo
to put the data in a specific named directory.Assuming you dumped data to a directory with the name "foo" if you try to dump data there again, it will fail. You can make it pass by adding
ARGS="--name foo --force"
which will clean the foo directory before dumping.Now run the command
This should reset your database to the initial data.
You should be able to load data from a directory by passing the directory name. Use the name from step 1.
If you don't pass a name or pass an invalid name the command will raise.
Checklist
#ISSUENUM
at the top of your PR to an existing open issue in the mozilla/addons repository.