Page MenuHomePhabricator

[Tunnelbroker] Add amqp client crate, add minimal usage
ClosedPublic

Authored by jon on Jun 11 2023, 6:58 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jun 13, 8:12 AM
Unknown Object (File)
Tue, Jun 11, 4:45 PM
Unknown Object (File)
Mon, Jun 10, 7:23 PM
Unknown Object (File)
Fri, May 24, 2:29 PM
Unknown Object (File)
Fri, May 24, 2:29 PM
Unknown Object (File)
Fri, May 24, 2:29 PM
Unknown Object (File)
Fri, May 24, 10:11 AM
Unknown Object (File)
Thu, May 23, 4:23 PM
Subscribers

Details

Summary

lapin seems to be the most commonly used. It is:

  • Still actively maintained
  • MIT licensed
  • Supports much of amqp v0.9.x

Part of https://linear.app/comm/issue/ENG-3716

Test Plan

N/A

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 11 2023, 7:01 PM
Harbormaster failed remote builds in B20169: Diff 27628!

New package looks good! Resigning so that one of the other reviews can take a look at the Rust

bartek added inline comments.
services/tunnelbroker/src/amqp.rs
9 ↗(On Diff #27628)

What's your policy on returning Result vs panicking?

Nit: We might want to return anyhow::Result<Connection> and return error, e.g. using with_context.
Anyway, it doesn't matter in the end - doing await? in main() results in panic anyway - it just might be more flexible.

This revision is now accepted and ready to land.Jun 12 2023, 9:02 AM

(Shellcheck CI job should pass after pulling in latest changes.)

services/tunnelbroker/src/amqp.rs
9 ↗(On Diff #27628)

A failure in connecting to amqp would be a critical failure. I'm fine with an early panic, and the executable exiting immediately in this case.

services/tunnelbroker/src/amqp.rs
9 ↗(On Diff #27628)

Makes sense, thanks