Page MenuHomePhabricator

[web] Import backup client
ClosedPublic

Authored by michal on Feb 12 2024, 7:21 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 1:08 PM
Unknown Object (File)
Tue, Nov 12, 8:43 AM
Unknown Object (File)
Wed, Nov 6, 5:34 PM
Unknown Object (File)
Fri, Nov 1, 8:51 PM
Unknown Object (File)
Oct 16 2024, 6:19 PM
Unknown Object (File)
Oct 16 2024, 12:33 PM
Unknown Object (File)
Oct 15 2024, 1:30 PM
Unknown Object (File)
Oct 15 2024, 1:30 PM
Subscribers

Details

Summary

Actually use the WASM backup client introduced in the previous diff.

  • webpack/keyserver machinery for downloading the wasm file
  • updated the worker init message so that it also intialized the backup client

Depends on D11041

Test Plan

Test on both:

  • yarn dev in web and keyserver
  • yarn prod, yarn prod-build

Send the worker init message, tested that there were no errors, and the WASM was correctly downloaded.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 12 2024, 7:26 AM
Harbormaster failed remote builds in B26762: Diff 37001!

Update keyserver dockerfile

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 13 2024, 3:33 AM
Harbormaster failed remote builds in B26784: Diff 37024!
ashoat added inline comments.
web/package.json
46 ↗(On Diff #37071)

In the case of the opaque-ke-wasm, we appear to be using a published package. Can you explain why you chose to take a different approach here? (It looks comm_query_executor.wasm also doesn't use a separate NPM package – worth discussing that in your response as well.)

web/package.json
46 ↗(On Diff #37071)

Modified the code, so there is no separate package and the approach is closer to comm_query_executor.wasm.

More detailed response about the chosen approach in the previous diff.

kamil requested changes to this revision.Feb 16 2024, 7:47 AM
kamil added inline comments.
keyserver/Dockerfile
129

I don't think we need this?

web/database/worker/db-worker.js
108

I think we should rename it

115

do we need localhost? I think this issue is related to the comment below with propper directory in webpack config

171

I think just undefined is enough without this hack - but maybe flow will complain

172
web/types/worker-types.js
37

perhaps we should make it more generic now?

web/webpack.config.cjs
159

Could you make sure it works on dev?

This revision now requires changes to proceed.Feb 16 2024, 7:47 AM

Fix the webpack issue and the urls pointing to the wasm file. Renamed databaseModuleFilePath. Removed unnecessary code in the dockerfile.

kamil added inline comments.
web/database/worker/db-worker.js
112 ↗(On Diff #37333)

isn't this enough?

166–173 ↗(On Diff #37333)

nit

174 ↗(On Diff #37333)

because this field is typed backupClientFilename?: ?string not backupClientFilename: undefined | string I think the suggested condition is better but maybe I am missing something

This revision is now accepted and ready to land.Feb 21 2024, 4:49 AM

Simplify

web/database/worker/db-worker.js
174 ↗(On Diff #37333)

null is a valid value for `backupClientFilename on dev. In this case we also want to init the backup client (with the default value).