-
Notifications
You must be signed in to change notification settings - Fork 5
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
Fix bug where validate_json
referential integrity check ignored documents in database
#861
Fix bug where validate_json
referential integrity check ignored documents in database
#861
Conversation
Props to @mbthornton-lbl for reporting this issue via Slack earlier today—thank you! |
I'm ready for this to be merged in, even in my absence (in case the reviews take place before I'm online). |
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 looks good. Possible future enhancements:
- We should also look at optimizations to queries / caching / memoization as a way to speed up multiple queries looking for the same doc.
- Seeing if we can do this as a transaction that can be rolled back.
Thanks, @shreddd! Regarding:
I want to expand on that by saying (as we discussed over Zoom earlier today) the transaction would allow us to "stage" the proposed operations on the real database (i.e. "perform" them, but without committing the transaction), do the referential integrity scan on that single database (on which the operations have been "staged"), and then roll back the transaction. I'll merge this in now in order to fix the bug currently present in the development environment. |
On this branch, I fixed a bug where the real-time referential integrity checker in the
validate_json
function was ignoring documents that were already in the database.Details
On this branch, I changed the real-time referential integrity checker code to no longer use the
OverlayDB
class. When I first wrote the real-time referential integrity checker code, I had made an incorrect assumption about how theOverlayDB
class worked.Instead of it using the
OverlayDB
class, I updated the checker to use the real database directly and to use the JSON payload directly. Checking now happens in two stages (one for each of those potential "residences" of each referenced document).The two stages are (in short—as a more detailed explanation is in the code comments):
With those changes in place, the checker no longer ignores documents that already exist in the database. I added a unit test that demonstrates this.
Related issue(s)
Fixes #860
Related subsystem(s)
docs
directory)Testing
I tested these changes by adding some unit tests that target them, and ensuring that those tests pass. I will leave it to GitHub Actions to tell me whether all pre-existing tests still pass.
Documentation
docs
directory)Maintainability
study_id: str
)# TODO
or# FIXME
black
to format all the Python files I created/modified