Paths

Table of Contentst

Differential D8177

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

Authored by • jon on Jun 11 2023, 6:58 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Sat, Nov 9, 4:19 AM

	Unknown Object (File)
	Fri, Nov 8, 6:38 AM

	Unknown Object (File)
	Mon, Nov 4, 10:56 AM

	Unknown Object (File)
	Mon, Nov 4, 10:55 AM

	Unknown Object (File)
	Mon, Nov 4, 10:55 AM

	Unknown Object (File)
	Mon, Nov 4, 10:55 AM

	Unknown Object (File)
	Sun, Nov 3, 9:58 PM

	Unknown Object (File)
	Sat, Nov 2, 2:01 PM

Subscribers

Details

Reviewers

varun
bartek
ashoat

Commits

rCOMM1d6de4d07114: [Tunnelbroker] Add amqp client crate, add minimal usage

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

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jon created this revision.Jun 11 2023, 6:58 PM

Herald added a subscriber: tomek. · View Herald TranscriptJun 11 2023, 6:59 PM

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!

• jon added a child revision: D8178: [Tunnelbroker] Use rabbitmq for message delivery.Jun 11 2023, 7:52 PM

• jon requested review of this revision.Jun 12 2023, 7:43 AM

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

bartek accepted this revision.Jun 12 2023, 9:02 AM

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.)

• jon added inline comments.Jun 12 2023, 10:07 AM

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.

Rebase on master

Harbormaster completed remote builds in B20202: Diff 27674.Jun 12 2023, 3:10 PM

bartek added inline comments.Jun 12 2023, 11:14 PM

services/tunnelbroker/src/amqp.rs
9 ↗	(On Diff #27628)	Makes sense, thanks

• jon marked an inline comment as done.Jun 15 2023, 10:56 AM

Closed by commit rCOMM1d6de4d07114: [Tunnelbroker] Add amqp client crate, add minimal usage (authored by • jon). · Explain WhyJun 19 2023, 8:13 PM

This revision was automatically updated to reflect the committed changes.

• jon added a commit: rCOMM1d6de4d07114: [Tunnelbroker] Add amqp client crate, add minimal usage.

Revision Contents
Changeset List

Path

Size

services/

tunnelbroker/

1 line

src/

14 lines

6 lines

2 lines

Diff 27674

services/tunnelbroker/Cargo.lock

Loading...

services/tunnelbroker/Cargo.toml

Loading...

services/tunnelbroker/src/amqp.rs

Loading...

services/tunnelbroker/src/config.rs

Loading...

services/tunnelbroker/src/main.rs

Loading...