Differential D9577

[Tunnelbroker] add recipient confirmation to Tunnelbroker
ClosedPublic
Actions

Authored by kamil on Oct 24 2023, 5:16 PM.

Tags

None

Referenced Files

None

Subscribers

Details

Reviewers

michal
bartek
varun
• jon

Commits

rCOMMd76df10f13b0: [Tunnelbroker] add recipient confirmation to Tunnelbroker

Summary

We should delete message from DDB after making sure that it was delivered and processed by recipient.

Depends on D9575

Test Plan

All Tunnelbroker tests passed
Empty DDB after tests (confirmation is processed by Tunnelbroker)
Tests exclusively for recipient confirmation in next diff

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kamil created this revision.Oct 24 2023, 5:16 PM

kamil held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptOct 24 2023, 5:16 PM

Herald added subscribers: will, tomek, ashoat. · View Herald Transcript

kamil added a child revision: D9578: [Tunnelbroker] add recipient confirmation tests.Oct 24 2023, 5:21 PM

Harbormaster completed remote builds in B23447: Diff 32347.Oct 24 2023, 5:34 PM

kamil published this revision for review.Oct 24 2023, 6:21 PM

kamil added inline comments.

services/commtest/tests/identity_tunnelbroker_tests.rs
95 ↗	(On Diff #32347)	I checked in the Identity logs, and it's true that this message was sent twice to the WebSocket (cc. @varun).

bartek added inline comments.Oct 25 2023, 10:54 AM

services/tunnelbroker/src/websockets/mod.rs
198–203 ↗	(On Diff #32347)	This way you can avoid unwrapping
services/tunnelbroker/src/websockets/session.rs
245–253 ↗	(On Diff #32347)	In the future we should do a batch delete operation here. We should have a follow up task for this. There are utilities in comm-services-lib for this but Tunnelbroker isn't using it yet.

address review

kamil added inline comments.Oct 31 2023, 1:37 PM

services/tunnelbroker/src/websockets/mod.rs
198–203 ↗	(On Diff #32347)	nice! thanks
services/tunnelbroker/src/websockets/session.rs
245–253 ↗	(On Diff #32347)	created ENG-5627 this will also require ENG-5172 - otherwise doesn't make a lot of sense

Harbormaster failed remote builds in B23598: Diff 32539!Oct 31 2023, 2:28 PM

Harbormaster completed remote builds in B23598: Diff 32539.Oct 31 2023, 3:22 PM

michal accepted this revision.Nov 2 2023, 12:59 PM

This revision is now accepted and ready to land.Nov 2 2023, 12:59 PM

Closed by commit rCOMMd76df10f13b0: [Tunnelbroker] add recipient confirmation to Tunnelbroker. · Explain WhyNov 6 2023, 4:29 PM

This revision was automatically updated to reflect the committed changes.

kamil added a commit: rCOMMd76df10f13b0: [Tunnelbroker] add recipient confirmation to Tunnelbroker.

bartek mentioned this in D12346: [commtest] Fix flaky identity-tunnelbroker test.Jun 7 2024, 8:07 AM

bartek mentioned this in rCOMM522d00e1ed6c: [commtest] Fix flaky identity-tunnelbroker test.Jun 12 2024, 2:31 PM

Revision Contents
Changeset List

Path

Size

services/

commtest/

src/

tunnelbroker/

7 lines

tests/

identity_tunnelbroker_tests.rs

19 lines

tunnelbroker/

src/

websockets/

4 lines

30 lines

Diff 32796

services/commtest/src/tunnelbroker/socket.rs

Loading...

services/commtest/tests/identity_tunnelbroker_tests.rs

Loading...

services/tunnelbroker/src/websockets/mod.rs

Loading...

services/tunnelbroker/src/websockets/session.rs

Loading...