Add feature flag for excluding redundant metadata in individual CVR reports #4036
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also: - Rename an existing feature flag for consistency with new flag - Make order of feature flags across relevant files consistent - Remove now unused REACT_APP_VX_SCREEN_ORIENTATION env var
When feature flag is active
arsalansufi
commented
Oct 4, 2023
| @@ -11,8 +11,8 @@ REACT_APP_VX_SKIP_CVR_ELECTION_HASH_CHECK=FALSE | |||
| REACT_APP_VX_SKIP_SCAN_ELECTION_HASH_CHECK=FALSE | |||
| REACT_APP_VX_SKIP_BALLOT_PACKAGE_AUTHENTICATION=FALSE | |||
| REACT_APP_VX_SKIP_CAST_VOTE_RECORDS_AUTHENTICATION=FALSE | |||
| REACT_APP_VX_DISABLE_CVR_ORIGINAL_SNAPSHOTS=FALSE | |||
| REACT_APP_VX_CAST_VOTE_RECORD_OPTIMIZATION_EXCLUDE_ORIGINAL_SNAPSHOTS=TRUE | |||
| REACT_APP_VX_CAST_VOTE_RECORD_OPTIMIZATION_EXCLUDE_REDUNDANT_METADATA=TRUE | |||
There was a problem hiding this comment.
As noted in the PR description:
I'm enabling both CVR optimization feature flags by default, since we'll want them enabled for real elections for the foreseeable future as far as I can tell. Faster performance on election day feels more important than hypothetical cross-system compatibility, for now
We'll of course want to revisit this come time for cert. Planning to make an issue so that we don't forget. But open to changing this now, too!
arsalansufi
requested review from
carolinemodic
and removed request for
a team and
carolinemodic
adghayes
reviewed
Oct 4, 2023
adghayes
approved these changes
Oct 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Easiest reviewed by commit
Overview
Issue link: #3995
In order to meet the VVSG2 requirements around continuous export, while also staying true to CDF, our export format has ended up a little strange.
We export a top-level
metadata.jsonfile for the whole export, plus acast-vote-record-report.jsonfile for every individual CVR. But we also repeat the metadata in themetadata.jsonfile in everycast-vote-record-report.jsonfile, so that the latter remain CDF-compliant. This PR introduces a feature flag to exclude that redundant metadata incast-vote-record-report.jsonfiles, significantly decreasing their size. This feature flag is similar in spirit to the existing feature flag for excluding original snapshots in CVRs.With a feature flag, we can toggle between efficiency and true CDF-compliance, as needed. Hopefully, something will come out of usnistgov/CastVoteRecords#45 such that this feature flag becomes unnecessary 🤞.
I'm enabling both CVR optimization feature flags by default, since we'll want them enabled for real elections for the foreseeable future as far as I can tell. Faster performance on election day feels more important than hypothetical cross-system compatibility, for now.
Demo Screenshot
An individual
cast-vote-record-report.jsonfile, before on the left, after on the right. All the extra data that's on the left but not on the right (i.e. the redundant metadata) is never used on import.Testing
Automated tests are coming in their own PR
Checklist
I have added logging where appropriate to any new user actions, system updates such as file reads or storage writes, or errors introducedN/A