Page MenuHomePhabricator

[Tunnelbroker] Remove usages of unwrap and expect

Authored by jon on May 12 2023, 3:12 PM.
Referenced Files
Unknown Object (File)
Sat, Feb 10, 8:23 AM
Unknown Object (File)
Sat, Feb 10, 8:22 AM
Unknown Object (File)
Sat, Feb 10, 8:22 AM
Unknown Object (File)
Sat, Feb 10, 8:13 AM
Unknown Object (File)
Sat, Feb 10, 7:56 AM
Unknown Object (File)
Jan 10 2024, 2:07 PM
Unknown Object (File)
Dec 16 2023, 8:48 AM
Unknown Object (File)
Dec 13 2023, 8:53 PM



Unwrap and expect were convenient for the happy cases. However,
they are unsatisfactory for actual edge cases as they cause panic which
puts tunnelbroker into a semi-broken state.

Depends on D7800

Test Plan
nix develop

# init localstack and run terraform
comm-dev services start
(cd services/terraform && ./

(cd services/tunnelbroker && cargo run &)
(cd services/commtest && cargo test --test tunnelbroker_integration_test)

Diff Detail

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.May 16 2023, 2:43 PM
bartek requested changes to this revision.May 17 2023, 10:09 PM

Requesting changes only to get answer about using comm-services-lib

16–19 ↗(On Diff #26424)

Maybe we could use types from comm-services-lib/

25–42 ↗(On Diff #26424)

Same as above, in comm-services-lib we have things as parse_string_attribute(), parse_number() etc... Perhaps they can be used here?

This revision now requires changes to proceed.May 17 2023, 10:09 PM
jon marked 2 inline comments as done.
jon added inline comments.
25–42 ↗(On Diff #26424)

I tried to use these utilities. But ran into some issues with imports. comm-services-lib still uses aws-sdk-dynamodb v0.24, while I'm currently using v0.27. They changed where AttributeValue is located.

In v0.24, AttributeValue is in dynamodb::model, but in v0.27 it's dynamodb::types. Although reverting would be pretty easy for this diff. It would be a big change for much of my other code.

I think the best path forward would be to update comm-services-lib, and then apply your suggestion.


Fair enough, accepting to unblock you now, but we definitely need to prioritize the comm-services-lib sync, or maintaining the lib will bring more costs than benefits. Anyway, this can be discussed in the Linear task you created ;)

This revision is now accepted and ready to land.May 30 2023, 6:30 AM
jon marked an inline comment as done.

Rebase on master and feedback in other diffs