[tunnelbroker] Use Hyper service instead of tokio_tungstenite
ClosedPublic
Actions

Authored by bartek on Sep 12 2023, 1:57 PM.

Details

Reviewers

kamil
michal
varun
• jon

Commits

rCOMM2ec7b28c4b41: [tunnelbroker] Use Hyper service instead of tokio_tungstenite

Summary

Replaces tokio_tungstenite usage with Hyper service that is implemented in D9158 - more detailed description in that diff.

Depends on D9158

Test Plan

Tunnelbroker builds and runs. Integration tests work, I'm able to connect to the tunnelbroker websocket port.
Connecting by HTTP works as well - the /health endpoint is available and returns 200 OK. Other paths return 404.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Sep 12 2023, 1:57 PM

bartek held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptSep 12 2023, 1:57 PM

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

bartek published this revision for review.Sep 12 2023, 2:12 PM

bartek mentioned this in D9158: [tunnelbroker] Hyper service for WebSocket connections.

Harbormaster completed remote builds in B22548: Diff 30994.Sep 12 2023, 2:20 PM

bartek added a child revision: D9165: [terraform] Add Tunnelbroker.Sep 12 2023, 2:32 PM

michal accepted this revision.Sep 13 2023, 6:21 AM

michal added inline comments.

services/tunnelbroker/src/websockets/mod.rs
96–99 ↗	(On Diff #30994)	We might want to move the channel creation into the `WebsocketService`. Now for example will still create the channel on `/heath` endpoint but we don't need to

This revision is now accepted and ready to land.Sep 13 2023, 6:21 AM

bartek added inline comments.Sep 13 2023, 7:19 AM

services/tunnelbroker/src/websockets/mod.rs
96–99 ↗	(On Diff #30994)	Yes, that's valid point. However, the problem is that `amqp_connection` isn't `Clone`, I had trouble with lifetimes while trying to move this reference into async service.