Skip to content

Type safety and code quality #51

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 16 commits into
base: devel
Choose a base branch
from
Open

Type safety and code quality #51

wants to merge 16 commits into from

Conversation

@MarekMracna
Copy link
Collaborator

This PR is focused on type safety and code quality. In particular, it makes these changes:

  1. The monolithic Task is now split into task phases. Methods now take their respective phases which removes the need for validation in many places.
  2. Devices and Tasks are now cached in State, reducing communication with the DB.
  3. Communicator is reworked - its only responsibility now is to relay protocol messages.
  4. The global mutex around State is removed, allowing for concurrent request handling.
  5. Async and locks are removed from tasks and moved to State.
  6. Developer documentation is added in the form of an ARCHITECTURE.md document.
  7. A lot of code is simplified and easier to reason about.

@MarekMracna
feat: create typestates for Finished, Failed and Declined tasks
e02d820 
Acknowledgements are moved into the FinishedTask typestate.
@MarekMracna
feat: create typestates for Voting and Running tasks
8340756
@MarekMracna
refactor: remove protocol round from failed proto::Task
4d17bef
@MarekMracna
refactor: store all necessary task information in TaskInfo
9037dc8 
This lets us pass around `Task` instances instead of always
hydrating from a task id or task model.
@MarekMracna
refactor: make task creation methods return Task instances
5c553ed
@MarekMracna
feat: cache devices in State
75476b8
@MarekMracna
refactor: return Task instances instead of TaskModels
35cd26c
@MarekMracna
feat: remove Communicator sharing, instead cache Tasks
00e7251 
This is a rather big rewrite with these main changes:
- A `RunningTask` now owns its `Communicator`
- `State` caches `Task`s
- The cache is populated lazily
- Refactor and cleanup of adjacent code
@MarekMracna
refactor: move format_task into Task
be63c16
@MarekMracna
feat: remove mutex around State
9fe191f 
This lets several requests to be served in parallel.
The locking granularity is now at Task level.
@MarekMracna
refactor: move Task persistence and caching into a TaskStore
83b4a8b
@MarekMracna
feat: persist active shares, remove decisions from Communicator
df688a9
@MarekMracna
doc: added ARCHITECTURE.md
8883019
@MarekMracna
refactor: fix nearly all clippy warnings
ce7b60f
@MarekMracna
refactor: remove the Group struct, use the Group model
0df0269 
The structures were basically the same.
Kristian had wanted to merge them already.
@MarekMracna
doc: update ARCHITECTURE.md to reflect recent commits
1ba709e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@MarekMracna