[services] Tunnelbroker - Disabling updating session state when session authentication is skipped
ClosedPublic
Actions

Authored by • max on Feb 16 2023, 7:06 AM.

Details

Reviewers

• jon
bartek

Commits

rCOMMe0bc3f7ac53c: [services] Tunnelbroker - Disabling updating session state when session…

Summary

This diff introduces disabling the session state updates when the session authentication mechanism is skipped by the config flag.
In this case, the sessions are not used, so we don't need to update their state.

Full context: ENG-2642
Linear task: ENG-3022

Test Plan

The DynamoDB database field for the session state is not updating when the config flag sessions.skip_authentication is used.

Diff Detail

Repository

rCOMM Comm

Branch

disable-update-session-online

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.Feb 16 2023, 7:06 AM

• max held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptFeb 16 2023, 7:06 AM

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

• max published this revision for review.Feb 16 2023, 7:12 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.

Harbormaster completed remote builds in B16556: Diff 22652.Feb 16 2023, 7:18 AM

bartek accepted this revision.Feb 20 2023, 4:09 AM

bartek added inline comments.

services/tunnelbroker/src/libcpp/Tunnelbroker.cpp
173–175	Just a nitpick, I have a feeling that this check is used often across the codebase (e.g. line 185 in this file), so extracting this to a separate function would be more concise and readable: bool shouldSkipAuthentication() { return comm::network::config::ConfigManager::getInstance().isParameterSet( comm::network::config::ConfigManager:: OPTION_SESSIONS_SKIP_AUTH_KEY); } // ... if (shouldSkipAuthentication()) { return; } But totally up to you

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

• max marked an inline comment as done.Feb 21 2023, 3:41 PM

• max added inline comments.

services/tunnelbroker/src/libcpp/Tunnelbroker.cpp
173–175	Just a nitpick, I have a feeling that this check is used often across the codebase (e.g. line 185 in this file), so extracting this to a separate function would be more concise and readable: bool shouldSkipAuthentication() { return comm::network::config::ConfigManager::getInstance().isParameterSet( comm::network::config::ConfigManager:: OPTION_SESSIONS_SKIP_AUTH_KEY); } // ... if (shouldSkipAuthentication()) { return; } But totally up to you It's used only twice and seems that is all usage occurrences. I prefer to live it as is because adding a function will add more complexity here and more lines of code. But thanks for a good suggestion @bartek!