-
Notifications
You must be signed in to change notification settings - Fork 37
Add NodeJS bindings to synapse_auto_compressor
#167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e5952da
83e853b
7c7186d
bbba5e6
a117017
e55b0c5
b98c741
9757a61
7d47f3d
25597cf
8ee0c73
5ac82e3
982bb4b
3604832
e4af53f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,4 +4,5 @@ | |
| *.old | ||
| **.sql | ||
| *.csv | ||
| **.log | ||
| **.log | ||
| .idea/ | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| # Running the compressor tools from nodejs | ||
|
|
||
| The automatic compressor tool is available through Node bindings. | ||
|
|
||
| Currently, the Node bindings wont produce logging. To see output, | ||
| look at the response array for chunk (room) specific results. | ||
|
|
||
| ## Requirements | ||
|
|
||
| NodeJS 16+ | ||
|
|
||
| ## Setting things up | ||
|
|
||
| 1. Navigate to the correct location | ||
|
|
||
| `$ cd synapse_auto_compressor` | ||
|
|
||
| 2. Build and install the library | ||
| `$ pnpm i && pnpm build` | ||
|
|
||
| ## Automatic tool example | ||
|
|
||
| ```node | ||
| const synapseAutoCompressor = require('synapse_auto_compressor'); | ||
|
|
||
| synapseAutoCompressor.runCompression( | ||
| "postgresql://localhost/synapse", | ||
| 500, | ||
| 100, | ||
| ).then(results => { | ||
| console.log(JSON.stringify(results)); | ||
| }); | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| .pnp.* | ||
| .yarn/ | ||
| synapse_auto_compressor.* | ||
| index.d.ts | ||
| index.js | ||
| node_modules/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,3 +10,7 @@ Bump the version number and run from the root directory of the repo: | |
| ``` | ||
| docker run -it --rm -v $(pwd):/io -e OPENSSL_STATIC=1 konstin2/maturin publish -m synapse_auto_compressor/Cargo.toml --cargo-extra-args "\--features='openssl/vendored'" | ||
| ``` | ||
|
|
||
| ## Publishing to NPM | ||
|
|
||
| TODO TBD | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this something you intend to PR after?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking in a follow-up pr, yes, if that is ok. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| // Build NodeJS bindings | ||
| fn main() { | ||
| napi_build::setup(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| { | ||
| "name": "synapse_auto_compressor", | ||
| "version": "0.1.4", | ||
| "description": "A tool to compress some state in a Synapse instance's database", | ||
| "main": "index.js", | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "git+ssh://git@github.com/matrix-org/rust-synapse-compress-state.git" | ||
| }, | ||
| "license": "Apache-2.0", | ||
| "files": [ | ||
| "index.d.ts", | ||
| "index.js" | ||
| ], | ||
| "napi": { | ||
| "binaryName": "synapse_auto_compressor", | ||
| "targets": [ | ||
| "x86_64-unknown-linux-gnu" | ||
| ] | ||
| }, | ||
| "engines": { | ||
| "node": ">= 12.22.0 < 13 || >= 14.17.0 < 15 || >= 15.12.0 < 16 || >= 16.0.0", | ||
| "pnpm": ">=8" | ||
| }, | ||
| "scripts": { | ||
| "build": "napi build --features node --platform --release", | ||
| "build:debug": "napi build --features node --platform" | ||
| }, | ||
| "devDependencies": { | ||
| "@napi-rs/cli": "^3.6.2", | ||
| "typescript": "^6.0.0" | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any objection to using pnpm instead?
On the team, we are more familiar with that, so if possible it's nice to keep our tools in alignment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have converted 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, trying to use the node bindings appears to be causing this now after converting to
pnpm:Unsure if this is due to the way I'm bundling the build locally to our nodejs app, but I suspect pnpm does something differently to the napi suggested yarn 🤔 Any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the yarn built node native binding is approx 3.3MB, while the one built by pnpm is ~700KB (+ a node_modules directory, which the yarn version doesn't generate).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually no, reverting to yarn doesn't seem to fix things, so broke something with the feature refactoring probably.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, after some amount of hair tearing, it seems I had dropped the
nodemodule declaration 🤦There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks and sorry for the trouble!