Page MenuHomePhabricator
Differential D9877

[terraform] [2/n] enable streams for DynamoDB identity-users table
ClosedPublic
Actions

Authored by will on Nov 13 2023, 11:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Apr 21, 4:51 AM
Unknown Object (File)
Thu, Apr 17, 9:25 PM
Unknown Object (File)
Thu, Apr 17, 1:49 PM
Unknown Object (File)
Thu, Apr 17, 11:24 AM
Unknown Object (File)
Thu, Apr 17, 10:40 AM
Unknown Object (File)
Thu, Apr 17, 9:22 AM
Unknown Object (File)
Thu, Apr 17, 6:11 AM
Unknown Object (File)
Thu, Apr 17, 5:36 AM
View All Files
Subscribers
ashoat
tomek

Details

Reviewers
bartek
varun
Commits
rCOMM0dc189b51a86: [terraform] [2/n] enable streams for DynamoDB identity-users table
Summary

Enable DynamoDB streams for the identity-users table

Depends on D9875

Test Plan

Verified on staging

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

will created this revision.Nov 13 2023, 11:36 PM
Herald added subscribers: tomek, ashoat. · View Herald TranscriptNov 13 2023, 11:36 PM
will retitled this revision from [terraform] enable streams for DynamoDB identity-users table to [terraform] [3/n] enable streams for DynamoDB identity-users table.Nov 13 2023, 11:43 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 13 2023, 11:52 PM
Harbormaster failed remote builds in B24097: Diff 33197!
will retitled this revision from [terraform] [3/n] enable streams for DynamoDB identity-users table to [terraform] [4/n] enable streams for DynamoDB identity-users table.Nov 20 2023, 6:03 PM
will retitled this revision from [terraform] [4/n] enable streams for DynamoDB identity-users table to [terraform] [3/n] enable streams for DynamoDB identity-users table.
will edited the summary of this revision. (Show Details)Nov 20 2023, 6:40 PM
will added a parent revision: D9876: [terraform] setting variables in main for opensearch.
will added a child revision: D9936: [services] [3/n] Rust opensearch indexing lambda.
will retitled this revision from [terraform] [3/n] enable streams for DynamoDB identity-users table to [terraform] [2/n] enable streams for DynamoDB identity-users table.Nov 21 2023, 12:14 AM
will edited the summary of this revision. (Show Details)
will added a parent revision: D9875: [terraform] [1/n] adding Opensearch service.
will updated this revision to Diff 33470.Nov 21 2023, 11:46 AM

rebase

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 21 2023, 12:25 PM
Harbormaster failed remote builds in B24315: Diff 33470!
will updated this revision to Diff 33474.Nov 21 2023, 1:29 PM

Rebase

Harbormaster completed remote builds in B24318: Diff 33474.Nov 21 2023, 1:59 PM
will requested review of this revision.Nov 21 2023, 1:59 PM
bartek mentioned this in D9958: [scripts] Fix terraform lint.Nov 23 2023, 2:44 AM
bartek accepted this revision.Nov 23 2023, 2:47 AM

I won't block you on this but could you please reformat the file before landing? 🙏

services/terraform/modules/shared/dynamodb.tf
102–105

This should have failed the lint but I just realized that scripts/terraform_pre_commit.sh is broken. I created D9958 to fix this.

Can you run terraform fmt to reformat this?

This revision is now accepted and ready to land.Nov 23 2023, 2:47 AM
will updated this revision to Diff 33742.Nov 27 2023, 7:20 AM

terraform fmt

Harbormaster completed remote builds in B24527: Diff 33742.Nov 27 2023, 10:45 AM
bartek mentioned this in rCOMMea2bcfa64137: [scripts] Fix terraform lint.Dec 1 2023, 12:20 AM
will updated this revision to Diff 34768.Dec 16 2023, 1:31 AM

Whitespace

Harbormaster completed remote builds in B25158: Diff 34768.Dec 16 2023, 2:35 AM
ashoat added a comment.Dec 17 2023, 9:26 AM

I'm not sure this is formatted correctly. @wyilio, you may need to rebase on master in order to pick up the commit where @bartek fixed the Terraform linter precommit hook

(Apologies if I'm wrong – don't fully understand Terraform formatting guidelines)

will planned changes to this revision.Dec 17 2023, 11:29 PM
will updated this revision to Diff 34812.Dec 18 2023, 12:45 PM

terraform fmt

This revision is now accepted and ready to land.Dec 18 2023, 12:45 PM
Harbormaster completed remote builds in B25195: Diff 34812.Dec 18 2023, 1:09 PM
Closed by commit rCOMM0dc189b51a86: [terraform] [2/n] enable streams for DynamoDB identity-users table (authored by will). · Explain WhyJan 5 2024, 3:25 PM
This revision was automatically updated to reflect the committed changes.
will added a commit: rCOMM0dc189b51a86: [terraform] [2/n] enable streams for DynamoDB identity-users table.

Revision Contents
Changeset List

PathSize
services/
terraform/
modules/
shared/
7 lines

Diff 33474

View Options

services/terraform/modules/shared/dynamodb.tf

// @flow // @flow
import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js'; import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';
import { isSQLiteSupported } from './utils/db-utils.js'; import { isDesktopSafari, isSQLiteSupported } from './utils/db-utils.js';
import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js'; import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';
import type { AppState } from '../redux/redux-setup.js'; import type { AppState } from '../redux/redux-setup.js';
import { import {
workerRequestMessageTypes, workerRequestMessageTypes,
type WorkerRequestMessage, type WorkerRequestMessage,
type WorkerResponseMessage, type WorkerResponseMessage,
} from '../types/worker-types.js'; } from '../types/worker-types.js';
Show All 16 Linesclass DatabaseModule {
status: DatabaseStatus; status: DatabaseStatus;
constructor() { constructor() {
const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous
? undefined ? undefined
: preloadedState.currentUserInfo?.id; : preloadedState.currentUserInfo?.id;
const isSupported = isSQLiteSupported(currentLoggedInUserID); const isSupported = isSQLiteSupported(currentLoggedInUserID);
if (!isSupported) { if (!isSupported || isDesktopSafari()) {
tomekUnsubmitted
Not Done
Inline Actions

Can we have a const instead of a function that tells if it is Safari? We can avoid having to detect the browser multiple times, as this fact doesn't change through the lifetime of the app.

tomek: Can we have a const instead of a function that tells if it is Safari? We can avoid having to…
kamilAuthorUnsubmitted
Done
Inline Actions

good point, thanks

kamil: good point, thanks
this.status = databaseStatuses.notSupported; this.status = databaseStatuses.notSupported;
kamilAuthorUnsubmitted
Done
Inline Actions

Naming might be confusing, but at this point database is not supported (we don't have a key to it), it will be supported after main thread will provide encryption key

kamil: Naming might be confusing, but at this point database is not supported (we don't have a key to…
tomekUnsubmitted
Not Done
Inline Actions

Do we change the status later?

tomek: Do we change the status later?
kamilAuthorUnsubmitted
Done
Inline Actions

yes, in initDBForLoggedInUser()

kamil: yes, in `initDBForLoggedInUser()`
} else { } else {
this.init(); this.init();
} }
} }
init(encryptionKey?: ?SubtleCrypto$JsonWebKey) { init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {
this.status = databaseStatuses.initInProgress; this.status = databaseStatuses.initInProgress;
this.worker = new SharedWorker(DATABASE_WORKER_PATH); this.worker = new SharedWorker(DATABASE_WORKER_PATH);
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines