Task rewrite: adopt AggregatorTask in datastore
#2017
Conversation
tgeoghegan
added
the
allow-changed-migrations
Adopts `janus_aggregator_core::task::test_util::NewTaskBuilder` and `janus_aggregator_core::task::AggregatorTask` in the `janus_aggregator_core::datastore` module. Much as the previous change provides two kinds of `Task` structure, we now provide two sets of methods for reading and writing tasks: one that deals in the new `AggregatorTask` and the other which deals in the old `Task`. We add routines for converting between `task::Task` and `task::AggregatorTask` to make it easier for these two paths through the datastore to co-exist. This conversion is lossy because `AggregatorTask` only retains one of the aggregator endpoints, but this doesn't cause substantial problems in Janus, and we can live it transitionally. Finally, the SQL schema for tasks is changed so that only the peer aggregator's endpoint is stored. Part of #1524
tgeoghegan
force-pushed
the
timg/new-task-datastore
branch
from
ea00c96 to
0bf9101
Compare
| } | ||
| } | ||
|
|
||
| impl From<AggregatorTask> for Task { |
There was a problem hiding this comment.
I'm cool with this for transitional purposes, but think we should do away with it ASAP (since it's lossy and contains false data). Looks like you're well on your way to doing so though.
There was a problem hiding this comment.
Yes, this is going to go away along with janus_aggregator_core::task::Task as this series of PRs progresses.
| self.time_precision, | ||
| self.tolerable_clock_skew, | ||
| self.hpke_keys.values().cloned().collect::<Vec<_>>(), | ||
| AggregatorTaskParameters::TaskProvHelper, |
There was a problem hiding this comment.
Elsewhere we've used Taskprov instead of TaskProv (i.e. lowercase P).
Whichever one works for me, but we should be consistent. It also wasn't introduced in this PR so doesn't have to be resolved here. Can proceed, then maybe a PR after the rest that does find | sed.
There was a problem hiding this comment.
That makes sense. I'm going to change the name in a later PR so that I don't have to rebase the existing stack.
For consistency with other renderings of "Taskprov". See previous discussion in [1]. [1]: #2017 (comment)
For consistency with other renderings of "Taskprov". See previous discussion in [1]. [1]: #2017 (comment)
For consistency with other renderings of "Taskprov". See previous discussion in [1]. [1]: #2017 (comment)
Adopts
janus_aggregator_core::task::test_util::NewTaskBuilderandjanus_aggregator_core::task::AggregatorTaskin thejanus_aggregator_core::datastoremodule. Much as the previous change provides two kinds ofTaskstructure, we now provide two sets of methods for reading and writing tasks: one that deals in the newAggregatorTaskand the other which deals in the oldTask.We add routines for converting between
task::Taskandtask::AggregatorTaskto make it easier for these two paths through the datastore to co-exist. This conversion is lossy becauseAggregatorTaskonly retains one of the aggregator endpoints, but this doesn't cause substantial problems in Janus, and we can live it transitionally.Part of #1524