-
Notifications
You must be signed in to change notification settings - Fork 26
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
Only reprocess batch tallies when relevant contest fields change #2062
Only reprocess batch tallies when relevant contest fields change #2062
Conversation
Previously, we were reprocessing all jurisdictions' batch tallies files whenever we made a request to update contests. This can be pretty disruptive because it affects every jurisdiction. There are a few cases that we know don't actually impact the batch tallies at all, so we can skip reprocessing: - When the audit admin doesn't change the contests at all (e.g. if they click "Save & Next" on the contests form without changing anything - When the audit admin only changes the contest's jurisdiction list. Since batch tallies are specific to a single jurisdiction, adding or removing a jurisdiction will have no impact on other jurisdiction's batch tallies.
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.
LGTM! Wishing I'd reviewed this last night 😅 as it would've come in handy today per all the reprocessing messages in #arlo-activity
|
||
def normalize_contest(contest): | ||
contest = contest.copy() | ||
# This field isn't sent from the client, its computed on the backend, so |
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.
super nit: it's
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.
Whoops, didn't get the email notification about this comment before merging 🤦 Doesn't feel worth a PR to fix
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.
No problem, I think I actually submitted 1 minute after your merge
] | ||
|
||
for contest in election.contests: | ||
db_session.delete(contest) |
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.
Why change this from the previous Contest.query.filter_by(election_id=election.id).delete()
?
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.
Accessing election.contests
on L348 just above causes the contests to be loaded into the ORM's cache, which caused a stale data error when later trying to delete and then commit the new contests. I honestly don't understand the error, but this fixed it and is functionally equivalent.
Fixes #1977
Previously, we were reprocessing all jurisdictions' batch tallies files whenever we made a request to update contests. This can be pretty disruptive because it affects every jurisdiction.
There are a few cases that we know don't actually impact the batch tallies at all, so we can skip reprocessing: