[blob-service] Introduce database client
ClosedPublic
Actions

Authored by bartek on Jul 9 2023, 2:03 PM.

Details

Reviewers

michal
varun
• patryk
• jon

Commits

rCOMM0f54000ebd83: [blob-service] Introduce database client

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:

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

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Jul 9 2023, 2:03 PM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptJul 9 2023, 2:03 PM

bartek added a child revision: D8451: [blob-service] DB: Add holder management methods.Jul 9 2023, 2:03 PM

Harbormaster completed remote builds in B20819: Diff 28506.Jul 9 2023, 2:23 PM

bartek edited the summary of this revision. (Show Details)Jul 9 2023, 10:55 PM

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

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.

bartek mentioned this in D8452: [blob-service] DB: Add remaining client methods.Jul 21 2023, 1:55 AM

Harbormaster completed remote builds in B21099: Diff 28910.Jul 21 2023, 2:14 AM

varun accepted this revision.Jul 23 2023, 7:34 AM

This revision is now accepted and ready to land.Jul 23 2023, 7:34 AM

Closed by commit rCOMM0f54000ebd83: [blob-service] Introduce database client (authored by bartek). · Explain WhyJul 25 2023, 1:44 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMM0f54000ebd83: [blob-service] Introduce database client.