Page MenuHomePhabricator

michal (Michał Gniadek)
User

Projects

User does not belong to any projects.

User Details

User Since
Sep 8 2022, 2:37 AM (54 w, 2 d)

Recent Activity

Yesterday

michal accepted D9246: [native][web] Fix keyserver store tranform error.

Is my understanding correct that there is still a toplevel connection field in redux state? Should we remove it inside of this migration?

Fri, Sep 22, 8:13 AM
michal added a comment to D9265: [lib/web/native] Add IntegrityStore.

I also tested another idea: let's do all "batch hashing" (where we hash all items of a store) on the keyserver side. So during login the keyserver would send client all thread hashes. The client would only update the hashes during thread ops. That means we should optimise the keyserver-side calculations, so we should do the schema conversion on the client. I tested it and got:

  • Do the hashing (without conversion) on the keyserver during login => I got ~0.5s for 7300 threads
  • From previous testing: updating one hash with conversion on native should take about ~10ms, but it should be a few threads at most at one time
Fri, Sep 22, 6:49 AM
michal requested review of D9267: [keyserver] Add new hashing to keyserver state check.
Fri, Sep 22, 6:20 AM
michal requested review of D9266: [lib/keyserver] Make Info generic mandatory in state sync specs.
Fri, Sep 22, 4:05 AM
michal requested review of D9265: [lib/web/native] Add IntegrityStore.
Fri, Sep 22, 4:03 AM
michal added a comment to D9256: [web/native] Add threadHashes to threadStore.

Alternative approach in D9265

Fri, Sep 22, 3:46 AM
michal added a comment to D9255: [native] Use threadInfo in setClientDBStoreActionType.

Alternative approach in D9265

Fri, Sep 22, 3:46 AM
michal planned changes to D9256: [web/native] Add threadHashes to threadStore.
Fri, Sep 22, 3:07 AM
michal planned changes to D9255: [native] Use threadInfo in setClientDBStoreActionType.
Fri, Sep 22, 3:07 AM
michal added a comment to D9262: [lib] Add `ThreadActivityStore` to Redux.

Yeah, you are correct. Basically if the value you would add previously put in website-responders is:

  • some default value than it should go to web/redux/default-state
  • value calculated on the keyserver than it should go in getInitialReduxStateResponder (+ some dummy default in default-state so flow doesn't complain)
Fri, Sep 22, 1:03 AM

Thu, Sep 21

michal requested review of D9256: [web/native] Add threadHashes to threadStore.
Thu, Sep 21, 8:14 AM
michal added a comment to D9255: [native] Use threadInfo in setClientDBStoreActionType.

Selector performance:
From looking at the code, we always select by state.threadStore.threadInfos, so there should be no issue. I've put up the next diff (D9256) which contains the actual changes to the threadStore: the newly added threadHashes are always updated at the same time as threadInfos inside of the threadStoreOpsHandlers.

Thu, Sep 21, 8:00 AM
michal accepted D9242: [services-lib] Use constant-time-eq for token verification.

Ok, seems like constant_time_eq checks the lengths inside, from looking at the code

Thu, Sep 21, 6:22 AM
michal requested review of D9255: [native] Use threadInfo in setClientDBStoreActionType.
Thu, Sep 21, 4:44 AM
michal requested review of D9253: [lib/keyserver] Improve state sync types.
Thu, Sep 21, 4:08 AM

Wed, Sep 20

michal added inline comments to D9217: [lib] Add logic for handling faonut actions.
Wed, Sep 20, 6:01 AM
michal accepted D9241: [services-lib] Add enum for authorization tokens.

Just some nits

Wed, Sep 20, 5:46 AM
michal accepted D9243: [services-lib] Support auth token in HTTP middleware.
Wed, Sep 20, 5:45 AM
michal added a comment to D9242: [services-lib] Use constant-time-eq for token verification.

I'm not really familiar with this stuff, so I might be completely wrong but:

Wed, Sep 20, 5:40 AM
michal accepted D9226: [lib] Update KeyserverCall type.

Nice!

Wed, Sep 20, 3:28 AM
michal added a comment to D9225: [lib] Create function for extracing keyserver id from object id.

This does not handle pending threads (more precisely sidebars created from a message). It's might be fine depending on the use case but I it feel like it should have a code comment in that case. Also agree with @tomek comments.

Wed, Sep 20, 3:17 AM
michal added a comment to D9217: [lib] Add logic for handling faonut actions.

Overall looks good but I have a question. In the test plan you write:

const searchMessagesActionObj: KeyserverCall<
  [SearchMessagesRequest],
  SearchMessagesResponse,
> = {
  actionFunc: searchMessages,
  config: { keyserverSelection: 'fanout' },
};

but shouldn't the return type be [SearchMessagesResponse] as it's a fanout action?

Wed, Sep 20, 3:10 AM
michal accepted D9143: [lib] Add memoization to useKeyserverCall.
Wed, Sep 20, 3:03 AM

Tue, Sep 19

michal added a comment to D9121: [lib] Create useKeyserverCall.

Made a task about validators ENG-4982 because I have more context on them.

Tue, Sep 19, 12:48 AM

Mon, Sep 18

michal accepted D9121: [lib] Create useKeyserverCall.

Unfortunate that we can't use ReturnType<T> and Parameters<T> from new versions of flow, might simplify it a little

Mon, Sep 18, 5:49 AM

Fri, Sep 15

michal updated the diff for D9109: [web] Enable db for all users.

Return instead of modifying

Fri, Sep 15, 6:38 AM
michal updated the diff for D9148: [web/keyserver] Handle different thread ids in url.

Fixes

Fri, Sep 15, 6:35 AM
michal updated the diff for D9141: [web] Use new intial redux state.

Fixes

Fri, Sep 15, 6:30 AM
michal updated the diff for D9109: [web] Enable db for all users.

Small fixes and early return

Fri, Sep 15, 5:44 AM
michal updated the diff for D9108: [web] Initialize database at startup on safari.

Set DatabaseModule fields to optional and add invariants

Fri, Sep 15, 5:38 AM
michal added inline comments to D9143: [lib] Add memoization to useKeyserverCall.
Fri, Sep 15, 3:57 AM

Thu, Sep 14

michal accepted D9200: [Tunnelbroker] add authentication tests.
Thu, Sep 14, 3:41 AM
michal accepted D8918: [Tunnelbroker] Authenticate connecting devices.
Thu, Sep 14, 3:40 AM

Wed, Sep 13

michal added inline comments to D9145: [web] Remove intial redux state from website responders.
Wed, Sep 13, 10:05 AM
michal added a comment to D9140: [web] Add setInitialReduxState action.

Improved error handling in D9141. It will bubble up any errors to the error boundary.

Wed, Sep 13, 9:49 AM
michal updated the diff for D9141: [web] Use new intial redux state.

Changed the component to function instead of arrow function. Added the error handling so any errors from the promise will bubble up to ErrorBoundary and display the "Something went wrong, please reload" page. We don't really have a any other good way to handle errors here.

Wed, Sep 13, 9:45 AM
michal added inline comments to D9108: [web] Initialize database at startup on safari.
Wed, Sep 13, 9:29 AM
michal updated the diff for D9108: [web] Initialize database at startup on safari.

Fixed window.status -> this.status issue

Wed, Sep 13, 9:29 AM
michal accepted D9176: [docker-compose] Match version tags with terraform.
Wed, Sep 13, 7:42 AM
michal accepted D9177: [terraform] Remove references to legacy services.
Wed, Sep 13, 7:27 AM
michal accepted D9165: [terraform] Add Tunnelbroker.

Would it make sense to add apply_immediately (at least for staging)? If I understand the documentation correctly, currently if we make changes to the user (or add broker configuration) we would have to reboot rabbitmq manually?

Wed, Sep 13, 7:02 AM
michal accepted D9162: [secrets] Generate RabbitMQ passwords.
Wed, Sep 13, 6:30 AM
michal accepted D9160: [tunnelbroker] Use Hyper service instead of tokio_tungstenite.
Wed, Sep 13, 6:21 AM
michal accepted D9158: [tunnelbroker] Hyper service for WebSocket connections.
Wed, Sep 13, 6:10 AM
michal accepted D7691: [Keyserver] Open websocket connection with tunnelbroker.
Wed, Sep 13, 6:03 AM
michal accepted D9156: [tunnelbroker] Make WebSocketSession generic.
Wed, Sep 13, 5:44 AM
michal accepted D9153: [tunnelbroker] Allow configuring AMQP credentials.
Wed, Sep 13, 5:44 AM
michal added a comment to D9156: [tunnelbroker] Make WebSocketSession generic.

Is generic-ness of this is going to be helpful in the future? Or is it going to be always HyperWebsocket under the hood? Because in that case we should just remove the generics and use HyperWebsocket directly

Wed, Sep 13, 1:33 AM
michal accepted D9155: [tunnelbroker] Don't listen on localhost only.
Wed, Sep 13, 1:19 AM
michal added inline comments to D9153: [tunnelbroker] Allow configuring AMQP credentials.
Wed, Sep 13, 12:54 AM
michal accepted D9139: [feature-flags] Use default HTTP port for deployments.

Should we also update the docker-compose.yml to use the new port?

Wed, Sep 13, 12:44 AM
michal accepted D9137: [services] Delete base-image and sandbox mode.

Nice!

Wed, Sep 13, 12:35 AM

Tue, Sep 12

michal added a comment to D7691: [Keyserver] Open websocket connection with tunnelbroker.

LGTM, mostly just a rename

Tue, Sep 12, 6:33 AM
michal requested review of D9148: [web/keyserver] Handle different thread ids in url.
Tue, Sep 12, 5:54 AM
michal requested review of D9147: [lib/web/native] Improve conversion utils.
Tue, Sep 12, 5:38 AM
michal requested review of D9146: [keyserver] Remove unused params in validation functions.
Tue, Sep 12, 5:29 AM
michal requested review of D9145: [web] Remove intial redux state from website responders.
Tue, Sep 12, 5:25 AM
michal requested review of D9141: [web] Use new intial redux state.
Tue, Sep 12, 3:50 AM
michal requested review of D9140: [web] Add setInitialReduxState action.
Tue, Sep 12, 3:39 AM
michal added a comment to D9124: [keyserver] Implement `getInitialReduxStateResponder`.

Probably moving policies logic from the responder itself to a higher place where we just avoid returning anything will make this code simpler - but since this will change in the future anyway (ENG-4137) it's okay the way it is

Tue, Sep 12, 2:03 AM
michal updated the diff for D9124: [keyserver] Implement `getInitialReduxStateResponder`.

Merge

Tue, Sep 12, 2:02 AM
michal updated the diff for D9122: [keyserver] Introduce getInitialReduxStateResponder.

Small fixes

Tue, Sep 12, 2:00 AM
michal added inline comments to D9122: [keyserver] Introduce getInitialReduxStateResponder.
Tue, Sep 12, 2:00 AM
michal requested review of D9109: [web] Enable db for all users.
Tue, Sep 12, 1:42 AM
michal updated the diff for D9109: [web] Enable db for all users.

Amend test plan, don't set userID if not needed. Also added a log for clearing sensitive data.

Tue, Sep 12, 1:41 AM
michal updated the test plan for D9109: [web] Enable db for all users.
Tue, Sep 12, 1:39 AM
michal added a comment to D9139: [feature-flags] Use default HTTP port for deployments.

Should we also update the docker-compose.yml to use the new port?

Tue, Sep 12, 12:34 AM
michal accepted D9138: [feature-flags] Standardize localstack and http config.
Tue, Sep 12, 12:27 AM

Mon, Sep 11

michal accepted D9128: [Tunnelbroker] match JS naming convention.
Mon, Sep 11, 11:32 PM
michal accepted D9104: [web] Fix nested a tags.
Mon, Sep 11, 7:11 AM
michal accepted D9125: [keyserver] fix fetching identity info.
Mon, Sep 11, 7:00 AM
michal accepted D9123: [web] Fix warnings on search screen.
Mon, Sep 11, 6:36 AM
michal requested review of D9124: [keyserver] Implement `getInitialReduxStateResponder`.
Mon, Sep 11, 6:30 AM
michal updated the summary of D9122: [keyserver] Introduce getInitialReduxStateResponder.
Mon, Sep 11, 6:19 AM
michal accepted D9104: [web] Fix nested a tags.

Thank you for fixing this!

Mon, Sep 11, 5:33 AM
michal accepted D9117: [feature-flags] Update Dockerfile.
Mon, Sep 11, 5:29 AM
michal accepted D9116: [blob-service] Update Dockerfile.
Mon, Sep 11, 5:28 AM
michal requested review of D9122: [keyserver] Introduce getInitialReduxStateResponder.
Mon, Sep 11, 4:21 AM
michal added inline comments to D9109: [web] Enable db for all users.
Mon, Sep 11, 1:25 AM
michal updated the diff for D9109: [web] Enable db for all users.

Rename shouldRestart, start db intialization on load, add comment for transform migration

Mon, Sep 11, 1:25 AM
michal updated the diff for D9108: [web] Initialize database at startup on safari.

Change initError to notSupported, move safari key init after setting status to initInProgress

Mon, Sep 11, 12:32 AM

Fri, Sep 8

michal updated the diff for D9110: [web] Migrate drafts to db for everyone.

Instead of removing the previous migration, make it noop.

Fri, Sep 8, 7:52 AM
michal requested review of D9111: [keyserver] Remove old current user info.
Fri, Sep 8, 7:52 AM
michal requested review of D9110: [web] Migrate drafts to db for everyone.
Fri, Sep 8, 7:12 AM
michal requested review of D9109: [web] Enable db for all users.
Fri, Sep 8, 6:58 AM
michal requested review of D9108: [web] Initialize database at startup on safari.
Fri, Sep 8, 6:07 AM
michal requested review of D9107: [web] Move safari db key init to database module.
Fri, Sep 8, 5:54 AM
michal requested review of D9105: [web] Hide databaseModule behind a function.
Fri, Sep 8, 4:20 AM

Thu, Sep 7

michal updated the diff for D9066: [backup] Setup terraform.

Change temporary docker repo to commapp

Thu, Sep 7, 2:30 AM
michal commandeered D9038: [terraform] Set up ECS Service Connect.
Thu, Sep 7, 2:07 AM

Tue, Sep 5

michal accepted D9078: [keyserver][lib][native][web] Remove lastCommunicatedPlatformDetails form redux top level.

LGTM, could you check that it still works with something like this:

Tue, Sep 5, 5:56 AM
michal accepted D9075: [client-backup] set backup-service prod URL.
Tue, Sep 5, 5:52 AM
michal accepted D9079: [lib][web][native] Start using lastCommunicatedPlatformDetailsSelector from keyserverStore.
Tue, Sep 5, 5:49 AM
michal accepted D9077: [keyserver][lib][native] Add lastCommunicatedPlatformDetails to keyserverStore.
Tue, Sep 5, 5:49 AM
michal accepted D9062: [web] Start persisting keyserverStore.
Tue, Sep 5, 5:46 AM
michal accepted D9082: [client-backup] exclude non-password users.
Tue, Sep 5, 5:29 AM
michal accepted D9080: [client-backup] rename `recentBackupHash` -> `mostRecentlyUploadedBackupHash`.
Tue, Sep 5, 5:26 AM
michal accepted D9038: [terraform] Set up ECS Service Connect.
Tue, Sep 5, 12:44 AM

Mon, Sep 4

michal accepted D9010: [client-backup] add local settings to store.
Mon, Sep 4, 8:34 AM
michal accepted D9003: [client-backup] implement backup handler.
Mon, Sep 4, 8:31 AM