Page MenuHomePhabricator

[blob-service] Introduce database client
ClosedPublic

Authored by bartek on Jul 9 2023, 2:03 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 6:10 PM
Unknown Object (File)
Fri, Nov 1, 6:10 PM
Unknown Object (File)
Fri, Nov 1, 6:10 PM
Unknown Object (File)
Fri, Nov 1, 6:09 PM
Unknown Object (File)
Fri, Nov 1, 5:46 PM
Unknown Object (File)
Sun, Oct 27, 4:35 PM
Unknown Object (File)
Mon, Oct 21, 10:43 AM
Unknown Object (File)
Oct 12 2024, 3:31 PM
Subscribers

Details

Summary

Part of ENG-4269.

Introduced a new DatabaseClient class to interface DDB.

Depends on D8443.

Test Plan

Tested together with 2 following diffs

Created a custom testing sandbox to play with these:

  1. Checkout this stack (this + 2 next diffs)
  2. Apply this patch and copy the tests.rs file contents attached there to the services/blob/src/database/client/tests.rs.
  3. cargo run

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek edited the test plan for this revision. (Show Details)
bartek published this revision for review.Jul 10 2023, 12:23 AM

otherwise the rest of the rust stuff looks fine to me.

Going to let varun check the DDB stuff

services/blob/src/database/client.rs
42–48 ↗(On Diff #28506)

Not the biggest fan of match, as it adds a lot of noise. However, the map style isn't much better. But I think is slightly more less noise.

This revision is now accepted and ready to land.Jul 10 2023, 10:44 AM

going to resign as I would like some of the others to review as well

This revision now requires review to proceed.Jul 10 2023, 10:45 AM
michal added 1 blocking reviewer(s): varun.

LGTM, but I'm not confident in db code, so adding for varun as blocking for this one

varun requested changes to this revision.Jul 12 2023, 4:02 PM
varun added inline comments.
services/blob/src/database/client.rs
157 ↗(On Diff #28506)

don't we need to loop here in case not all the rows are returned?

This revision now requires changes to proceed.Jul 12 2023, 4:02 PM

Opted out of introducing batch methots for now - they require some more testing and changes.
Hopefully, we don't need them right now - they'll be used in the cleanup task.
I'll handle this in a separate diff later to unblock this stack.

This revision is now accepted and ready to land.Jul 23 2023, 7:34 AM
This revision was automatically updated to reflect the committed changes.