Skip to content

Conversation

@windoverwater
Copy link
Collaborator

@windoverwater windoverwater commented Apr 25, 2023

At a high level this PR contains the following work:

  • add support for the web-api interface to function (various odds and ends)
  • split the original implementation of using the logging module for BOTH logging and printing (which was probably a bad thing but that is history) into logging and a wrapper around either printing to STDOUT or printing to data structure which can be directly returned to the web-api. This is to initially support passing the verify-ballot, tally-election, and show-contest text/log/json data back to the web-api. The idea is to just get it done for the demo and to refactor it later
    • the print wrapper was added to the Operation class for convenience. Again the thought is that this can be refactored after the demo
    • Ion and I talked about this but never did anything about it - I picked a random design and implemented it here now
  • changed ElectionConfig from being a single global value (which does not support the web-api model) to a cache with one level. This will allow the CLI demo to still run as is (the cache should be hot for the entire CLI demo) while also allowing the web-api demo to run (across multiple different git workspaces in parallel). But this needs to be reviewed carefully as I may not be understanding how FastAPI is handling async endpoints.
  • removed the super().init calls from the ops package constructors as they become redundant

Creating this PR early so there is time to potentially address issues in case it wants to be included in the release branch.

@windoverwater windoverwater linked an issue Apr 25, 2023 that may be closed by this pull request
@windoverwater windoverwater self-assigned this Apr 25, 2023
@windoverwater windoverwater added the enhancement New feature or request label Apr 25, 2023
@windoverwater
Copy link
Collaborator Author

@ion-oset @stratofax I know you guys are busy, but I would like to have plenty of time to address any comments to this PR before the code freeze. And this PR has been open for a while. Said another way, if there are no comments today, let me know if it is NOT ok to merge this as is with a comment here. I would to merge it by tomorrow.

ion-oset
ion-oset previously approved these changes Apr 29, 2023
Copy link
Contributor

@ion-oset ion-oset left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍🏼. Mostly minor comments.

@ion-oset
Copy link
Contributor

All your fixes LGTM.

@windoverwater windoverwater merged commit e06d657 into main May 1, 2023
@windoverwater windoverwater deleted the feature/78-4th-and-5th-endpoint-work branch May 1, 2023 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support the 4th and 5th endpoints

3 participants