Paths

Table of Contentst

[services] Tunnelbroker - Update `SessionItem` database entity with the `IsOnline` field
ClosedPublic
Actions

Authored by • max on Sep 16 2022, 11:45 AM.

Details

Reviewers

• jon
varun
tomek
• karol

Commits

rCOMM556c31433ce5: [services] Tunnelbroker - Update `SessionItem` database entity with the…

Summary

This diff introduces adding of IsOnline field to the DynamoDB database in the DeviceSessionItem entity.
To send notifications if the device is offline we need to distinguish the device state. To do this we will ping the device and change the device status (online/offline) in this database in this field.

This diff is a part of the stack.
More context is available in the Linear task description.

Related Linear task: ENG-1766

Test Plan

Successfully built.
Run yarn run-unit-tests and yarn run-integration-tests.

Diff Detail

Repository

rCOMM Comm

Branch

update-session-with-online

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• max created this revision.Sep 16 2022, 11:45 AM

• max held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptSep 16 2022, 11:45 AM

Herald added subscribers: • abosh, • karol, atul and 2 others. · View Herald Transcript

Harbormaster failed remote builds in B12226: Diff 16774!Sep 16 2022, 11:46 AM

• max added a child revision: D5165: [services] Tunnelbroker - Add `updateSessionItemIsOnline` database method.Sep 16 2022, 11:51 AM

• max edited the summary of this revision. (Show Details)Sep 16 2022, 12:18 PM

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

• max added reviewers: varun, tomek.

• max edited the summary of this revision. (Show Details)Sep 16 2022, 12:20 PM

• max mentioned this in D5165: [services] Tunnelbroker - Add `updateSessionItemIsOnline` database method.

• max published this revision for review.Sep 16 2022, 12:25 PM

• max mentioned this in D5166: [services] Tunnelbroker - Add test for the `updateSessionItemIsOnline` database method.

Herald added a reviewer: • karol. · View Herald TranscriptSep 16 2022, 12:26 PM

Harbormaster completed remote builds in B12226: Diff 16774.Sep 16 2022, 12:52 PM

Passed a unit test in D5166.

Unfortunately, DatabaseManager tests are being filtered out because there's a requirement on external services. I don't have integration tests yet added.

This revision now requires changes to proceed.Sep 16 2022, 1:21 PM

• max added a child revision: D5175: [services] Tunnelbroker - Update device online status in `Get` handler.Sep 19 2022, 5:58 AM

• max removed a child revision: D5175: [services] Tunnelbroker - Update device online status in `Get` handler.

In D5164#151290, @jon wrote:

Passed a unit test in D5166.

Unfortunately, DatabaseManager tests are being filtered out because there's a requirement on external services. I don't have integration tests yet added.

It makes sense to change the test description to use a Docker. I've changed it. Thanks, @jon !

tomek accepted this revision.Sep 19 2022, 6:19 AM

• jon accepted this revision.Sep 19 2022, 11:08 AM

This revision is now accepted and ready to land.Sep 19 2022, 11:08 AM

Rebased on changes. Default false value was added for an online status on a new session insert.

Harbormaster failed remote builds in B12310: Diff 16898!Sep 20 2022, 5:58 AM

Rebase on a master changes.

This revision was landed with ongoing or failed builds.Sep 20 2022, 6:19 AM

Closed by commit rCOMM556c31433ce5: [services] Tunnelbroker - Update `SessionItem` database entity with the… (authored by • max). · Explain Why

This revision was automatically updated to reflect the committed changes.

• max added a commit: rCOMM556c31433ce5: [services] Tunnelbroker - Update `SessionItem` database entity with the….

• max mentioned this in rCOMMe4f0d2ebe260: [services] Tunnelbroker - Add `updateSessionItemIsOnline` database method.

• max mentioned this in rCOMMf6cc2d93dede: [services] Tunnelbroker - Add test for the `updateSessionItemIsOnline` database….

Harbormaster failed remote builds in B12314: Diff 16902!Sep 20 2022, 8:47 AM

Tunnelbroker unit tests seem to be failing?

In D5164#152455, @ashoat wrote:

Tunnelbroker unit tests seem to be failing?

It was unrelated to these changes:

'flake.nix', searching up
do you want to allow configuration setting 'extra-substituters' to be set to 'https://comm.cachix.org' (y/N)? # Received cancellation signal,

The prompt issue was fixed with https://phab.comm.dev/D5194. For CI we need to "give permission" ahead of time to receive the comm binary cache. The default behavior is to prompt a user, but for CI, there is no user.