[SEA-NodeJS] (2/8) napi-rs binding consumer — TS loader + build script#380
[SEA-NodeJS] (2/8) napi-rs binding consumer — TS loader + build script#380msrathore-db wants to merge 6 commits into
Conversation
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
84d0860 to
20231c8
Compare
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
20231c8 to
2f1865b
Compare
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Addresses PR #380 review findings C1, C2 (partial), C3, H1, H2, H3, H4, H5, H6, H7, H8 (runtime guard), H9, M1, M2, M3, M4 (linguist), M5, M6, M7, M8, L1, L2, L4. - C1 lint: drop `.js` ext from native/sea require so eslint import/extensions passes; gitignore the auto-generated index.js and *.node artifacts; prettier-ignore the napi-rs auto-generated index.d.ts / index.js. - C2 publish: whitelist native/sea/index.{js,d.ts} in .npmignore; declare @databricks/sea-native-linux-x64-gnu in optionalDependencies. The kernel-side package-name rename (drop the linux-x64-gnu prefix) is tracked separately as a cross-PR ask. - C3 test wiring: move tests/native/version.test.ts -> tests/unit/sea/, tests/native/e2e-smoke.test.ts -> tests/e2e/sea/; both now picked up by the existing mocharc globs and run via the existing `npm test` / `npm run e2e` jobs. - H1 noise drop: version.test.ts now asserts one meaningful semver check; three tautological assertions removed. - H2 + H3 + H7 lazy load: rewrite SeaNativeLoader.ts on the lib/utils/lz4.ts pattern. Lazy require behind getSeaNative(); capability-detection helper tryGetSeaNative(); structured error messages that classify MODULE_NOT_FOUND vs ERR_DLOPEN_FAILED and include platform/arch/Node-version + install hint. - H4 supply chain: pin @napi-rs/cli to 2.18.4 in devDependencies; build:native switches to `npx --no-install` so the pinned local install is used (no per-build network fetch). - H5 path: switch build:native default kernel path to the canonical `../../databricks-sql-kernel/napi-binding` (the worktree-specific `-sea-WT` suffix is gone). - H6 CI safety: e2e suite hard-fails when CI=true and any required env var is missing; dev machines still skip. - H8 runtime guard: loader throws a structured error on Node <18 instead of attempting a dlopen that would fail mysteriously. Driver's engines.node stays >=14 — Thrift consumers on older Node continue to work. - H9 + M1 + M2 types: tsconfig adds a `@sea-native` path alias to native/sea/index.d.ts; loader imports the real Connection / Statement / ConnectionOptions / ExecuteOptions / ArrowBatch / ArrowSchema types and re-exports them. Tests use the re-exported types — the inline-shape duplication across three files collapses. - M3 README: rewrite native/sea/README.md to match kernel reality. The napi crate is a standalone Cargo workspace (not a pyo3 sibling); explain the tls-rustls choice that the standalone workspace exists to enable. - M4 drift detection: mark native/sea/index.d.ts as linguist-generated in .gitattributes so GitHub collapses it in diffs and excludes from blame/language stats. - M5 artifacts: gitignore native/sea/index.js, index.node, index.*.node. - M6 + M7 e2e coverage: decode the IPC payload via apache-arrow's tableFromIPC and assert numRows + cell value (not just shape); add drain-past-null idempotence and schema-before-fetch coverage. - L1 build scripts: collapse build:native + build:native:debug into one script taking BUILD_PROFILE (defaults to --release). - L2 / L4 / M8: drop the version() alias and the "Round 2+ will…" comment debt; the loader now actually delivers the value the prior JSDoc was only promising. Verified on this branch: npm run lint clean (0 errors); npm run prettier clean for PR-owned files (3 unrelated pre-existing warnings on PR #378 territory); tsc --project tsconfig.build.json clean; mocha on tests/unit/sea passes (4/4); mocha on tests/e2e/sea passes against a live pecotesting warehouse (2/2, IPC decode confirms SELECT 1 returns 1).
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
Creates the napi-rs binding skeleton: Cargo.toml + lib.rs + module stubs for database/connection/statement/result/error/logger. Captures napi-rs tokio Handle via OnceCell in runtime.rs. Single working #[napi] fn version() proves the binding loads + executes end-to-end in Node. Depends on krn-async-public-api branch (path dep on kernel). Round 2 will add open/execute/fetch methods. Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
…wired
Adds real async methods on the opaque wrappers backing M0:
- openSession (free function) with PAT → kernel Session
- Connection::execute_statement → kernel ExecutedStatement
- Statement::fetch_next_batch / schema / cancel / close → kernel ResultStream
- Arrow batches returned as IPC bytes (per Layer 2 design)
- Error mapping preserves kernel ErrorCode + SQLSTATE for TS layer
- All entry points wrapped in catch_unwind
End-to-end smoke test against pecotesting passes.
No new dependencies beyond arrow-{ipc,array,schema} + futures.
Uses kernel async public API (no block_on).
Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
…indings Round 1 scaffold declared tracing + tracing-subscriber as deps but never used them. Removed. Logger bridge will re-add in round 3. Other findings from 6b3affd-2026-05-15.md reviewed: - Finding 2 (Database::Drop unreachable in Round 1b) — obsoleted by Round 2 (40d0b57): database.rs no longer declares a Database struct or Drop impl; it is now an `open_session` free function. - Finding 3 (empty Connection::Drop) — obsoleted by Round 2: the Drop impl now spawns a real fire-and-forget close on the captured tokio handle. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Per D-006 architectural decision (Python team's workspace pattern):
all language bindings (PyO3, napi-rs) now live as workspace siblings
in the kernel repo at databricks-sql-kernel/{pyo3,napi}/.
What this commit removes from the nodejs repo:
- native/sea/Cargo.toml (path dep relocated; package now at
databricks-sql-kernel/napi/Cargo.toml with path = "..")
- native/sea/build.rs
- native/sea/src/* (lib, runtime, database, connection, statement,
result, error, logger, util — all 9 files)
- native/sea/package.json (the @databricks/sea-native-linux-x64-gnu
sub-package moves to the kernel workspace too)
- native/sea/index.js (regenerated artifact)
What stays in nodejs:
- native/sea/index.d.ts — TS declarations consumed by lib/sea/ adapter
- native/sea/README.md (new) — explains the move; points readers at
databricks-sql-kernel/napi/
What's updated:
- package.json: `build:native` and `build:native:debug` scripts now
delegate to the kernel workspace via $DATABRICKS_SQL_KERNEL_REPO
(defaults to ../../databricks-sql-kernel-sea-WT/napi-binding for the
local dev worktree layout). Build copies index.node + index.d.ts
back into native/sea/ for the loader to find.
Why workspace co-location:
- Arrow version pinning lockstep — no silent IPC version drift
- path = ".." (clean) vs ../../../../databricks-sql-kernel-sea-WT/...
- Single CI: cargo build --workspace covers kernel + pyo3 + napi
- Kernel API changes that break either binding caught at PR-review time
- Future cgo binding for Go SEA slots in as another workspace member
This branch (sea-napi-binding) is now a thin consumer of the kernel
napi crate. The actual Rust code lives at krn-napi-binding HEAD on
the kernel repo (commit debe3d7).
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
…generated Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Addresses PR #380 review findings C1, C2 (partial), C3, H1, H2, H3, H4, H5, H6, H7, H8 (runtime guard), H9, M1, M2, M3, M4 (linguist), M5, M6, M7, M8, L1, L2, L4. - C1 lint: drop `.js` ext from native/sea require so eslint import/extensions passes; gitignore the auto-generated index.js and *.node artifacts; prettier-ignore the napi-rs auto-generated index.d.ts / index.js. - C2 publish: whitelist native/sea/index.{js,d.ts} in .npmignore; declare @databricks/sea-native-linux-x64-gnu in optionalDependencies. The kernel-side package-name rename (drop the linux-x64-gnu prefix) is tracked separately as a cross-PR ask. - C3 test wiring: move tests/native/version.test.ts -> tests/unit/sea/, tests/native/e2e-smoke.test.ts -> tests/e2e/sea/; both now picked up by the existing mocharc globs and run via the existing `npm test` / `npm run e2e` jobs. - H1 noise drop: version.test.ts now asserts one meaningful semver check; three tautological assertions removed. - H2 + H3 + H7 lazy load: rewrite SeaNativeLoader.ts on the lib/utils/lz4.ts pattern. Lazy require behind getSeaNative(); capability-detection helper tryGetSeaNative(); structured error messages that classify MODULE_NOT_FOUND vs ERR_DLOPEN_FAILED and include platform/arch/Node-version + install hint. - H4 supply chain: pin @napi-rs/cli to 2.18.4 in devDependencies; build:native switches to `npx --no-install` so the pinned local install is used (no per-build network fetch). - H5 path: switch build:native default kernel path to the canonical `../../databricks-sql-kernel/napi-binding` (the worktree-specific `-sea-WT` suffix is gone). - H6 CI safety: e2e suite hard-fails when CI=true and any required env var is missing; dev machines still skip. - H8 runtime guard: loader throws a structured error on Node <18 instead of attempting a dlopen that would fail mysteriously. Driver's engines.node stays >=14 — Thrift consumers on older Node continue to work. - H9 + M1 + M2 types: tsconfig adds a `@sea-native` path alias to native/sea/index.d.ts; loader imports the real Connection / Statement / ConnectionOptions / ExecuteOptions / ArrowBatch / ArrowSchema types and re-exports them. Tests use the re-exported types — the inline-shape duplication across three files collapses. - M3 README: rewrite native/sea/README.md to match kernel reality. The napi crate is a standalone Cargo workspace (not a pyo3 sibling); explain the tls-rustls choice that the standalone workspace exists to enable. - M4 drift detection: mark native/sea/index.d.ts as linguist-generated in .gitattributes so GitHub collapses it in diffs and excludes from blame/language stats. - M5 artifacts: gitignore native/sea/index.js, index.node, index.*.node. - M6 + M7 e2e coverage: decode the IPC payload via apache-arrow's tableFromIPC and assert numRows + cell value (not just shape); add drain-past-null idempotence and schema-before-fetch coverage. - L1 build scripts: collapse build:native + build:native:debug into one script taking BUILD_PROFILE (defaults to --release). - L2 / L4 / M8: drop the version() alias and the "Round 2+ will…" comment debt; the loader now actually delivers the value the prior JSDoc was only promising. Verified on this branch: npm run lint clean (0 errors); npm run prettier clean for PR-owned files (3 unrelated pre-existing warnings on PR #378 territory); tsc --project tsconfig.build.json clean; mocha on tests/unit/sea passes (4/4); mocha on tests/e2e/sea passes against a live pecotesting warehouse (2/2, IPC decode confirms SELECT 1 returns 1). Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
6b70ce4 to
548a14b
Compare
Stack
Linear stack of 8 PRs landing the M0 + M1 Phase 1 SEA NodeJS work. Merge in order from base ↑ to tip. The tip branch (
msrathore/sea-auth-u2m, PR #383) is the single snapshot containing everything in flight — point your test or benchmark harness at it for an end-to-end check.sea-abstractionsea-napi-binding.nodeartifactsea-errors-loggingErrorCode→ JS error-class mapping (M0 minimum)sea-authuseSEA: truesea-executionexecuteStatement+openSession(sessionConfig, initialCatalog/Schema)sea-resultssea-operationsea-auth-u2m← TIPCompanion kernel stack (
databricks/databricks-sql-kernel): 8 PRs — root #26 (async-public-api) → #27 → #25 → #29 → #28 → #30 → #24 → #23 (tip).Policy: new PRs always stack on the current tip. No sibling/parallel topology. No force-pushes on existing PRs unless absolutely necessary; if a PR's content is wrong, add a fix-up commit on top of the stack tip rather than rewriting history.
This PR is position 2/8.
Summary
Thin TypeScript loader for the napi-rs binding (
SeaNativeLoader), generatednative/sea/index.d.ts, and thebuild:nativenpm script that compiles against the kernel workspace'snapi/sub-crate.Companion kernel PR
The Rust napi crate itself lives in the kernel repo — see
databricks/databricks-sql-kernel#25(Database / Connection / Statement / ResultStream classes) and its skeleton predecessor #27.Test plan
npm run build:nativeproducesnative/sea/index.linux-x64-gnu.noderequire()s the platform-specific binaryDraft until you give the go for review.
Co-authored-by: Isaac