LB-225: Add support for importing data dumps. #274
Conversation
paramsingh
changed the title
|
Added tests too. |
paramsingh
force-pushed
the
import-data-dumps
branch
from
580b57a
to
7ecd7ae
Compare
|
Rebase, please |
paramsingh
force-pushed
the
import-data-dumps
branch
2 times, most recently
from
ab1e9f3
to
ce03530
Compare
|
I would love opinions on the problem I mentioned in the OP. The stats dump currently cannot be imported alone, it needs a corresponding private dump.
|
|
Oh sorry, didn't see that question:
That is exactly how MB does it -- lets do that too. |
paramsingh
force-pushed
the
import-data-dumps
branch
from
ce03530
to
6a63060
Compare
|
Rebased over #293 and added logic to only import user table from stats dump when needed. |
paramsingh
force-pushed
the
import-data-dumps
branch
2 times, most recently
from
ee2c37d
to
a555f57
Compare
Also, Add more error catching and logging
paramsingh
force-pushed
the
import-data-dumps
branch
from
a555f57
to
56a8231
Compare
listenbrainz/db/dump.py
Outdated
| pxz = subprocess.Popen(pxz_command, stdout=subprocess.PIPE) | ||
|
|
||
| # create a transaction and start importing | ||
| with db.engine.begin() as connection: |
There was a problem hiding this comment.
I'm glad you decided to do this in a transaction, which at first glance is the opposite of importing. :) However, there are subtle differences -- namely when importing you can't have a live site running. That isn't a valid or desired use case. Therefore there is no need to run inside a transaction -- in fact doing a full data import in a transaction could cause some interesting problems. So, please remove this transaction.
Based off #272, so will need to be rebased after that.
There are a bunch of things more to do here, for example the stats table has a foreign key to the user table so the stats tables can't be imported alone, it needs a corresponding entry in the user table. Not sure how to solve this, other than sanitizing users and putting it in the stats dump too.