| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
First of three stacked PRs splitting the SEA foundation (was the single 8/8 PR #383). This PR establishes a SEA-backed connection and session: - SeaBackend: connect() validates auth + captures the napi ConnectionOptions; openSession() folds catalog/schema/sessionConf and opens a kernel session. - SeaAuth: PAT + OAuth M2M + OAuth U2M validation/routing (mirrors the DBSQLClient auth-validation pattern; slash-prepended httpPath via prependSlash). - SeaErrorMapping: kernel ErrorCode → JS error-class mapping. - SeaSessionBackend: session open/close. executeStatement + metadata methods throw a clear deferred error — wired in [2/3] SEA execution + results. - DBSQLClient: route `useSEA: true` to the real SeaBackend (with IClientContext). - native/sea: the napi-rs binding surface (.d.ts + router); the .node stays gitignored (CI does not build it, loader/version tests skip when absent). Tests: PAT / M2M / U2M / edge-case auth suites, kernel error mapping, and the DBSQLClient SEA-routing + partial-init guard. Drops the obsolete stub SeaBackend.test (real backend is covered by the auth suites). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
|
🔴 P0 (Critical) — verified at source, must fix SEA is non-functional on a clean install: the generated loader requires package names that are garbled AND undeclared in optionalDependencies. I confirmed both halves:
→ Fix: regenerate native/sea/index.js with the correct npmName, declare the real per-platform packages in optionalDependencies, and align the loader hint / .npmignore comments to that name. Add a test 🟠 P1 (Important)
🟡 P2 (Minor)
|
Sorry, something went wrong.
- P0 packaging: the napi router's per-platform npm names were garbled — the M0 triple was baked into the prefix for every platform (@databricks/sea-native-linux-x64-gnu-<triple>, and the doubled ...-gnu-linux-x64-gnu). Corrected to the canonical @databricks/sql-kernel-<triple> (matches the loader hint + native/sea/README). Added native-packaging.test to lock the naming. The per-platform packages are unpublished, so they remain undeclared in optionalDependencies (npm ci can't resolve an unpublished pin); README documents the M0 build:native load path. - Moved @napi-rs/cli out of optionalDependencies (a build tool should not land in consumer installs); build:native now fetches it on demand via npx --yes. - P1 auth-flow: documented honestly that SEA's OAuth flow selection DIVERGES from Thrift — Thrift keys off the secret (DBSQLClient.ts:216), SEA keys off oauthClientId presence — including the two real gaps (id+no-secret throws; U2M has no custom client id). Fixed the stale DBSQLClient.ts:143 ref. - P1 stale ref: SeaErrorMapping pointed at DBSQLOperation.ts:209; the Canceled switch is now ~:374. Updated. - P2 context: SeaBackendOptions.context is now required (was an `as IClientContext` downcast of undefined → latent NPE). Tests pass a shared makeFakeContext(). - P2 prependSlash: dropped the dead lib/utils/prependSlash.ts (nothing imported it; SeaAuth and DBSQLClient each keep their own inline copy). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
|
Thanks @gopalldb — validated all of these against the source; addressed in the latest push (f6418e0, plus the rebased stack). Summary: 🔴 P0 — packaging
🟠 P1
🟡 P2
None of these were already fixed downstream; the fixes live in #409 and propagate through the rebased #410/#411. This pull request and its description were written by Isaac. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
First of three stacked PRs splitting the SEA foundation (replaces the single 8/8 #383). Establishes a SEA-backed connection + session.
Tests: PAT/M2M/U2M/edge-case auth, kernel error mapping, DBSQLClient SEA routing + partial-init guard.
Stack: 1/3 → #410 → #411
This pull request and its description were written by Isaac.