-
Notifications
You must be signed in to change notification settings - Fork 694
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
trying to fix integrity errors before migrating tables #12279
trying to fix integrity errors before migrating tables #12279
Conversation
Build Artifacts
|
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.
One tweak to avoid premature Django initialization, which I also think will help with the testing error.
kolibri/utils/main.py
Outdated
@@ -185,6 +186,27 @@ def _copy_preseeded_db(db_name, target=None): | |||
) | |||
|
|||
|
|||
def sqlite_check_foreign_keys(): | |||
db_connection = connections["default"] |
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.
This is explicitly being done in the method that is before Django setup, so I think using the django DB connection here might not be a good idea. Could you instead generate the connection directly using the Python sqlite library?
We should also iterate over all the SQLite databases listed in https://github.com/learningequality/kolibri/blob/develop/kolibri/deployment/default/sqlite_db_names.py#L14 to be safe.
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.
argg, got it. Thanks for the advice
Changed as requested
c4c94d7
to
132e318
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.
Code looks right, I'll test it with the database that @AllanOXDi had with the original issue. Thanks!
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.
Manual testing with an affected database shows that the fix is applied and logged, and manual inspection of the database afterwards shows that the affected rows have been removed.
Summary
Before updating kolibri database on a Django upgrade, check integrity of the tables and fix automatically errors that can be safely fixed
References
Closes: #12138
Reviewer guidance
Using a broken db, check that it's fixed.
Using a good db, check no problems are added
Testing checklist
PR process
Reviewer checklist
yarn
andpip
)