Page MenuHomePhabricator

[identity] Add function to remove holders for devices
ClosedPublic

Authored by bartek on Oct 9 2024, 1:34 AM.
Tags
None
Referenced Files
F3339506: D13650.id45165.diff
Thu, Nov 21, 7:58 PM
F3339253: D13650.id45000.diff
Thu, Nov 21, 7:23 PM
F3338542: D13650.id45149.diff
Thu, Nov 21, 6:18 PM
F3335966: D13650.diff
Thu, Nov 21, 1:04 PM
Unknown Object (File)
Wed, Nov 20, 9:13 AM
Unknown Object (File)
Wed, Nov 20, 9:13 AM
Unknown Object (File)
Sat, Nov 9, 3:55 AM
Unknown Object (File)
Sat, Nov 9, 2:22 AM
Subscribers

Details

Summary

Created a blob submodule in Identity for blob service actions.
Created function that calls Blob remove-multiple-holders endpoint for all holders prefixed by given device IDs.
Used exponential backoff to retry eventual failed holders.

Depends on D13648, D13649, D13643

Test Plan

Took a test from D13648 test plan, moved to Identity and called the new function in place of the direct BlobServiceClient call

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Oct 9 2024, 2:29 AM
bartek added inline comments.
services/identity/src/comm_service/blob.rs
32–35

This causes instant failure return when await? returns err, ignoring exponential backoff.
I'm wondering if sth like this wouldn't be better:

let remaining = match blob_client.remove_multiple_holders(request.clone()).await {
    Ok(response) => response.failed_requests.into(),
    Err(_) => request,
  };

So we retry on HTTP call failure too.

kamil added inline comments.
services/identity/src/comm_service/blob.rs
26

looks like needs to be updated following https://phab.comm.dev/D13648#inline-77564

32–35

I think this makes more sense

This revision is now accepted and ready to land.Oct 10 2024, 2:34 AM

Address feedback - retry RPC fails too