-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
make sure parallel corpora exist before updating #565
Conversation
f8e6250
to
8745642
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #565 +/- ##
==========================================
+ Coverage 62.96% 62.97% +0.01%
==========================================
Files 279 279
Lines 13979 13984 +5
Branches 1814 1814
==========================================
+ Hits 8802 8807 +5
Misses 4551 4551
Partials 626 626 ☔ View full report in Codecov by Sentry. |
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.
Where is this update error coming from? I just wonder if it'd be cleaner to avoid the operation if there are no parallel corpora or otherwise change it.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ddaspit)
When a DataFile is being updated, it syncs the filename to the corpora and parallelCorpora. The old records in the database don't have parallel-corpora, and when the update is pushed, it cannot filter because the field (parallelCorpora) does not exist. This adds it on startup (migration) ensuring that the update operation can work. |
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.
At some point, we probably want to store a model version in the DB, so that we only run the migration if we need to.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @johnml1135)
@Enkidu93 We did not mark the new |
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.
OK, yep, that's a good point.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @johnml1135)
This is remove the migration scripts: #567 |
Fix for #564
This change is