Conversation
Looks like historically we have allowed our DBs to hold broken references - basically a failure to cascade remove in any of our multiple API versions. The current API raises a RuntimeError when this occurs but to me it seems so common, that we should better handle it. And the way I am handling it here is to just skip the record with the broken reference. The implementation at the moment goes a bit in circles by adding the item and then removing it if a broken ref is found. This is because to be able to check references at the moment we need to add the item.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #566 +/- ##
==========================================
- Coverage 84.94% 84.88% -0.06%
==========================================
Files 80 80
Lines 10923 10929 +6
Branches 1599 1602 +3
==========================================
- Hits 9278 9277 -1
- Misses 1281 1286 +5
- Partials 364 366 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Will this hide broken records in the database? Should we give users a heads-up instead? |
Yes, it will. Users won't know about it - but should they? It is not something they introduced, it was us. I feel the best is to skip the records as unfortunate mistakes from the past - rather than asking users to go remove them using a SQL client in order to be able to use their DBs. Or I'm missing something? Edit: Sorry, what I'm trying to say is at the moment, users would need to use an external tool such as DB browser to remove the offending records in order to use their DBs. I don't think that's an option for us. I am advocating for skipping the broken records but I'm aware that might not be optimal. This is just a quick fix so users (and by users I mean myself) can keep using their old DBs without pain. |
|
@soininen I will just merge this as is since it solves the basic problem. Perhaps we can discuss more later on about a more thorough solution. |
|
I've seen some broken database records as well, but the cases have been rare -- last one was maybe a year or two ago. Having strict If these issues are common enough to warrant a repair tool, then that could be solution. Not sure why it should be integrated into purge, though. Anyway, I opened a new issue about this: #569 I am open to other solutions, too. |
3 participants
Looks like historically we have allowed our DBs to hold broken references - basically a failure to cascade remove in any of our multiple API versions.
The current API raises a RuntimeError when this occurs but to me it seems so common, that we should better handle it. And the way I am handling it here is to just skip the record with the broken reference. The implementation at the moment goes a bit in circles by adding the item and then removing it if a broken ref is found. This is because to be able to check references at the moment we need some side effects that happen when we add the item.
Edit: just fixed the implementation so it doesn't go on circles anymore. We validate the references before adding the item to the in-memory mapping - it was possible.
Checklist before merging