Page MenuHomePhabricator
Differential D6624

[services] Tunnelbroker - Disabling messages persistence on messages send
ClosedPublic
Actions

Authored by • max on Feb 6 2023, 3:11 AM.
Tags
None
Referenced Files
F3247714: D6624.id22121.diff
Fri, Nov 15, 6:27 AM
Unknown Object (File)
Thu, Nov 7, 1:30 AM
Unknown Object (File)
Sat, Oct 26, 9:55 PM
Unknown Object (File)
Wed, Oct 23, 12:40 AM
Unknown Object (File)
Oct 15 2024, 9:26 AM
Unknown Object (File)
Oct 15 2024, 9:08 AM
Unknown Object (File)
Oct 15 2024, 8:36 AM
Unknown Object (File)
Oct 11 2024, 3:21 PM
View All Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
• jon
bartek
Commits
rCOMMf95747ecac53: [services] Tunnelbroker - Disabling messages persistence on messages send
Summary

This diff introduces changes to the Tunnelbroker gRPC messages stream implementation according to the ENG-2641 task, where we need to disable messages persistence in the DynamoDB database using the config file parameter from D6622.
In this diff, we are disabling saving messages into the database when calling send messages, and a config flag from D6622 is presented.

Linear task: ENG-2641

Test Plan
  1. CI gates are passed.
  2. Manual testing: The client connects to the gRPC messages stream and sends the messages to the certain deviceID. The expected result is messages are not stored in the messages table DynamoDB database when the config flag messages.skip_persistence is presented.

Diff Detail

Repository
rCOMM Comm
Branch
disable-persistence-onsend
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• max created this revision.Feb 6 2023, 3:11 AM
• max held this revision as a draft.
Herald added a reviewer: • jon. · View Herald TranscriptFeb 6 2023, 3:11 AM
Herald added subscribers: atul, tomek, ashoat. · View Herald Transcript
• max added a parent revision: D6623: [services] Tunnelbroker - Disabling checking of the persisted messages on stream connect.Feb 6 2023, 3:12 AM
Harbormaster failed remote builds in B16170: Diff 22120!Feb 6 2023, 3:16 AM
• max updated this revision to Diff 22121.Feb 6 2023, 4:07 AM

Fixing building error.

Harbormaster completed remote builds in B16171: Diff 22121.Feb 6 2023, 4:25 AM
• max added a child revision: D6625: [services] Tunnelbroker - Extracting `deleteQueueIfEmpty` logic to a separate function.Feb 6 2023, 4:52 AM
• max edited the summary of this revision. (Show Details)Feb 6 2023, 5:27 AM
• max edited the test plan for this revision. (Show Details)
• max added a reviewer: bartek.
• max published this revision for review.Feb 6 2023, 5:45 AM
• jon requested changes to this revision.Feb 7 2023, 9:01 AM
• jon added inline comments.
services/tunnelbroker/src/libcpp/Tunnelbroker.cpp
260 ↗(On Diff #22121)

looks like a typo

This revision now requires changes to proceed.Feb 7 2023, 9:01 AM
• max planned changes to this revision.Feb 8 2023, 5:41 AM
• max updated this revision to Diff 22290.Feb 9 2023, 2:49 AM

Fixing space.

• max marked an inline comment as done.Feb 9 2023, 2:50 AM
• max added inline comments.
services/tunnelbroker/src/libcpp/Tunnelbroker.cpp
260 ↗(On Diff #22121)

looks like a typo

Thanks, @jon! It's weird that the CI was passed with this space.

Harbormaster completed remote builds in B16291: Diff 22290.Feb 9 2023, 3:03 AM
ashoat added inline comments.Feb 9 2023, 8:37 AM
services/tunnelbroker/src/libcpp/Tunnelbroker.cpp
260 ↗(On Diff #22121)

Can you investigate why? Unless this is valid Rust syntax, it sounds like something is wrong with CI. If that's the case, would be good to create a Linear task to investigate

• max marked 2 inline comments as done.Feb 10 2023, 11:13 AM
• max added inline comments.
services/tunnelbroker/src/libcpp/Tunnelbroker.cpp
260 ↗(On Diff #22121)

Can you investigate why? Unless this is valid Rust syntax, it sounds like something is wrong with CI. If that's the case, would be good to create a Linear task to investigate

I've tested it, It was built successfully locally, so this is not a CI issue.

bartek accepted this revision.Feb 10 2023, 10:16 PM
bartek added inline comments.
services/tunnelbroker/src/libcpp/Tunnelbroker.cpp
260 ↗(On Diff #22121)

Whitespaces next to :: is a valid Rust syntax. It must be valid to accept newlines and tabs when splitting very long expressions.

ashoat added inline comments.Feb 12 2023, 4:52 PM
services/tunnelbroker/src/libcpp/Tunnelbroker.cpp
260 ↗(On Diff #22121)

Ah, thanks for explaining

• jon accepted this revision.Feb 13 2023, 5:10 PM

Whitespaces next to :: is a valid Rust syntax. It must be valid to accept newlines and tabs when splitting very long expressions.

I'm used to C++ just making this into undefined behavior; a landmine for later.

This revision is now accepted and ready to land.Feb 13 2023, 5:10 PM
• max updated this revision to Diff 22979.Feb 23 2023, 4:41 AM
• max marked an inline comment as done.

Rebasing on master and parent changes.

Harbormaster completed remote builds in B16810: Diff 22979.Feb 23 2023, 4:56 AM
Closed by commit rCOMMf95747ecac53: [services] Tunnelbroker - Disabling messages persistence on messages send (authored by • max). · Explain WhyFeb 23 2023, 5:00 AM
This revision was automatically updated to reflect the committed changes.
• max added a commit: rCOMMf95747ecac53: [services] Tunnelbroker - Disabling messages persistence on messages send.

Revision Contents
Changeset List

PathSize
services/
tunnelbroker/
src/
libcpp/
8 lines

Diff 22120

View Options

services/tunnelbroker/src/libcpp/Tunnelbroker.cpp

Loading...