[identity] Add function to remove holders for devices
ClosedPublic
Actions

Authored by bartek on Oct 9 2024, 1:34 AM.

Details

Reviewers

varun
will
kamil

Commits

rCOMMf54b7e5ef580: [identity] Add function to remove holders for devices

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 created this revision.Oct 9 2024, 1:34 AM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptOct 9 2024, 1:34 AM

bartek added a child revision: D13651: [identity] Pass Blob client to AuthenticatedService.Oct 9 2024, 1:36 AM

Harbormaster completed remote builds in B32130: Diff 45000.Oct 9 2024, 2:07 AM

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 ↗	(On Diff #45000)	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 accepted this revision.Oct 10 2024, 2:34 AM

kamil added inline comments.

services/identity/src/comm_service/blob.rs
26 ↗	(On Diff #45000)	looks like needs to be updated following https://phab.comm.dev/D13648#inline-77564
32–35 ↗	(On Diff #45000)	I think this makes more sense