[services] Tunnelbroker - Skipping sending notif token request when notifications are off by the config flag
ClosedPublic
Actions

Authored by • max on Feb 23 2023, 5:41 AM.

Details

Reviewers

• jon
bartek

Commits

rCOMM266b7425934b: [services] Tunnelbroker - Skipping sending notif token request when…

Summary

This diff skipping sending new notif token requests to the bidirectional stream when the notifications are off by the notifications.disable config flag parameter. We don't need to send this message to the stream and should skip checking the existing notification token in this case.

Linear task: ENG-2639

Test Plan

Add the following config parameter to the tunnelbroker.ini file:

[notifications]
disable = true

Connect to the Tunnelbroker's message stream.
The expected result should be no newNotifyTokenRequired message in any case.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• max created this revision.Feb 23 2023, 5:41 AM

• max held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptFeb 23 2023, 5:41 AM

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

• max added a parent revision: D6490: [services] Tunnelbroker - Add disable notifications config parameter.Feb 23 2023, 5:41 AM

Harbormaster completed remote builds in B16819: Diff 22993.Feb 23 2023, 5:53 AM

• max published this revision for review.Feb 23 2023, 6:11 AM

• max edited the summary of this revision. (Show Details)

• max edited the test plan for this revision. (Show Details)

• max added a reviewer: bartek.

bartek accepted this revision.Feb 23 2023, 6:20 AM

bartek added inline comments.

services/tunnelbroker/src/server/mod.rs
189–192 ↗	(On Diff #22993)	Still nitpicking about readability, but up to you ;)

This revision is now accepted and ready to land.Feb 23 2023, 6:20 AM

• max marked an inline comment as done.Feb 23 2023, 6:32 AM

• max added inline comments.

services/tunnelbroker/src/server/mod.rs
189–192 ↗	(On Diff #22993)	Still nitpicking about readability, but up to you ;) I would prefer to not create an additional variable here, thanks for commenting ;)