Depends on D5008
Adding initialize function to the blob put client.
Details
Details
backup builds
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| services/backup/blob_client/src/put_client.rs | ||
|---|---|---|
| 82–85 ↗ | (On Diff #16243) | Is it possible for sender to have a different type than receiver? It would be strange... so maybe there's a way to have a type parameter provided only once? |
| 91–92 ↗ | (On Diff #16243) | Can we directly use Result? |
| 146–171 ↗ | (On Diff #16243) | This code can be simplified significantly by replacing response_present variable with a simple break statement |
| 173–176 ↗ | (On Diff #16243) | Not really sure whether introducing maybe_response is a good idea. Creating it causes an error reported twice: once when we create it and match Err and then here. |
| 199–201 ↗ | (On Diff #16243) | Can we update this in a diff that introduced this comment? |
| services/backup/blob_client/src/put_client.rs | ||
|---|---|---|
| 82–85 ↗ | (On Diff #16243) | |
| 91–92 ↗ | (On Diff #16243) | right |
| 146–171 ↗ | (On Diff #16243) | I agree it can, although I wouldn't call it a significant simplification, it makes this code thinner by just some words. For me, the current version is clearer but I don't mind changing it. |
| 173–176 ↗ | (On Diff #16243) | Ehh, yes, that's right. This one time I wanted to make the rust code less indented, but ok, will change. |
| 199–201 ↗ | (On Diff #16243) | yes |
| services/backup/blob_client/src/put_client.rs | ||
|---|---|---|
| 82–85 ↗ | (On Diff #16243) | Have you tried something like this? let (request_thread_tx, mut request_thread_rx) = mpsc::channel::<PutRequestData>(MPSC_CHANNEL_BUFFER_CAPACITY); |
| 91–92 ↗ | (On Diff #16243) | Can we do this in other places too? |
| 199–201 ↗ | (On Diff #16243) | I can see that D5004 contains updated values, but this diff still shows this as changed, so the diff should be probably rebased. |
| services/backup/blob_client/src/put_client.rs | ||
|---|---|---|
| 82–85 ↗ | (On Diff #16243) | This worked |