aggregator API: idempotent task creation #1549
Merged
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
| "task with same VDAF verify key and task ID already exists with different parameters".to_string(), | ||
| Status::Conflict, | ||
| ); | ||
| return Err(datastore::Error::User(err.into())); |
There was a problem hiding this comment.
We need to match on the error received by map_err below for datastore::Error::User, try downcasting to this module's error type, and return that unwrapped error instead if it succeeds. Otherwise, duplicate tasks would just get the "Error storing task" message.
There was a problem hiding this comment.
Good catch, I forgot to write a test for the task mutation case.
There was a problem hiding this comment.
Fixed: I did this to avoid writing identical match arms for the _ case and the case where datastore::Error:User wraps something besides aggregator_api::Error. Too clever?
divergentdave
approved these changes
Jun 30, 2023
branlwyd
approved these changes
Jul 3, 2023
tgeoghegan
force-pushed
the
timg/aggregator-api-bearer-token-default
branch
from
5422828
to
8b4e9ea
Compare
tgeoghegan
force-pushed
the
timg/agg-api-task-idempotence
branch
from
8a53c17
to
a97613f
Compare
tgeoghegan
force-pushed
the
timg/aggregator-api-bearer-token-default
branch
3 times, most recently
from
69a9546
to
c85a508
Compare
tgeoghegan
force-pushed
the
timg/agg-api-task-idempotence
branch
from
a97613f
to
31f73dd
Compare
Base automatically changed from
timg/aggregator-api-bearer-token-default
to
main
August 1, 2023 23:00
Makes the task creation endpoint in the aggregator API idempotent, by checking whether a task corresponding to the incoming `PostTaskReq` already exists. If it does, and all the other task's parameters match the `PostTaskReq`, then we return 201 Created. If the task exists but some other parameter (e.g., min batch size) differs, we return 409 Conflict. Deletion was already correctly handled so no new changes or test were needed. Resolves #1507
tgeoghegan
force-pushed
the
timg/agg-api-task-idempotence
branch
from
31f73dd
to
4a5a9d5
Compare
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.
Makes the task creation endpoint in the aggregator API idempotent, by checking whether a task corresponding to the incoming
PostTaskReqalready exists. If it does, and all the other task's parameters match thePostTaskReq, then we return 201 Created. If the task exists but some other parameter (e.g., min batch size) differs, we return 409 Conflict.Deletion was already correctly handled so no new changes or test were needed.
Resolves #1507