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
Details
Details
- Reviewers
marcin bartek kamil - Commits
- rCOMM43658727a042: [web] Import backup client
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
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| 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. |
| keyserver/Dockerfile | ||
|---|---|---|
| 129 ↗ | (On Diff #37263) | I don't think we need this? |
| web/database/worker/db-worker.js | ||
| 108 ↗ | (On Diff #37263) | I think we should rename it |
| 115 ↗ | (On Diff #37263) | do we need localhost? I think this issue is related to the comment below with propper directory in webpack config |
| 171 ↗ | (On Diff #37263) | I think just undefined is enough without this hack - but maybe flow will complain |
| 172 ↗ | (On Diff #37263) | |
| web/types/worker-types.js | ||
| 37 ↗ | (On Diff #37263) | perhaps we should make it more generic now? |
| web/webpack.config.cjs | ||
| 159 ↗ | (On Diff #37263) | Could you make sure it works on dev? |
Comment Actions
Fix the webpack issue and the urls pointing to the wasm file. Renamed databaseModuleFilePath. Removed unnecessary code in the dockerfile.
| 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 |
Comment Actions
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). |