Refactor public function in dump_manager #200
Conversation
|
Do tests in #199 pass after these changes are applied? |
critiquebrainz/data/dump_manager.py
Outdated
|
|
||
|
|
||
| def create_base_archive(dump_dir, temp_dir): | ||
| """Creates a dump of all license-independent information: (users, license).""" |
There was a problem hiding this comment.
This docstring should be improved. Add information about what this function takes as arguments and what it returns.
critiquebrainz/data/dump_manager.py
Outdated
| print("Done!") | ||
|
|
||
|
|
||
| def create_base_archive(dump_dir, temp_dir): |
There was a problem hiding this comment.
I feel like it would be better for readability and decoupling if we created a temp_dir in this function instead of passing it.
critiquebrainz/data/dump_manager.py
Outdated
| REVISION_COMBINED_SQL = """ | ||
| SELECT {columns} FROM revision JOIN review | ||
| def create_reviews_archive(dump_dir, temp_dir, license_id=None): | ||
| """Creates a dump of reviews filtered on the given license_id, their revisions and |
There was a problem hiding this comment.
This docstring should be improved as well. You should also add what happens when license_id=None.
| @@ -19,7 +20,6 @@ | |||
| import critiquebrainz.db.review as db_review | |||
| import critiquebrainz.db.moderation_log as db_moderation_log | |||
| from critiquebrainz.frontend.external.musicbrainz_db.entities import get_multiple_entities, get_entity_by_id | |||
| from langdetect import detect | |||
There was a problem hiding this comment.
This change doesn't seem related to anything in this PR.
critiquebrainz/data/dump_manager.py
Outdated
| with open(os.path.join(base_archive_tables_dir, 'license'), 'w') as f: | ||
| cursor.copy_to(f, 'license', columns=_TABLES["license"]) | ||
| except Exception: | ||
| print('Error while copying tables') |
There was a problem hiding this comment.
You should log the error as well. Also, the message should be something like error while copying tables during creation of base archive so that it is easier to understand where the error took place.
Refactor public function in
dump_manager.py(Removing a lot of duplicate code) and using transaction for copying tables.