Skip to content

Commit 4856e49

Browse files
peco-engineer-bot[bot]peco-engineer-bot[bot]
committed
ai: apply changes for #457 (3 review threads)
Addresses: - #3663057241 at CONNECTION_PARAMETERS.md:60 - #3663100673 at CONNECTION_PARAMETERS.md:108 - #3663100689 at CONNECTION_PARAMETERS.md:62 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 0836b8a commit 4856e49

3 files changed

Lines changed: 24 additions & 23 deletions

File tree

‎CONNECTION_PARAMETERS.md‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ column.
5757
| `authType: 'access-token'` + `token` (PAT) | ✅ | ✅ | |
5858
| `authType: 'databricks-oauth'` — M2M (`oauthClientId` + `oauthClientSecret`) | ✅ | ✅ | Kernel runs OIDC discovery + client-credentials internally. |
5959
| `authType: 'databricks-oauth'` — U2M (browser) | ✅ | ⚠️ | See U2M gaps below. |
60-
| `oauthScopes` | ✅ | ✅ | Kernel default U2M scopes = `['sql','offline_access']` (Thrift parity); M2M = `['all-apis']`. |
61-
| `oauthClientId` (U2M) | ✅ | ❌ | **Kernel-side gap.** Kernel U2M hardcodes `client_id` and **rejects** a custom `oauthClientId`; Thrift honors it. |
62-
| `oauthClientId` + no secret | ✅ (U2M) | ❌ | **Divergence.** Thrift routes to U2M with that id; kernel keys the flow off `oauthClientSecret` presence and throws an M2M "secret required" error. |
60+
| `oauthScopes` | ❌ | ✅ | **Thrift ignores `oauthScopes`** — `createAuthProvider` never threads it into `DatabricksOAuth`, so `authenticate()` always falls back to `defaultOAuthScopes` (`['sql','offline_access']`). Only the kernel honors a custom `oauthScopes`; its defaults happen to match Thrift's fallback (U2M = `['sql','offline_access']`, M2M = `['all-apis']`). |
61+
| `oauthClientId` (U2M) | ✅ | ✅ | The kernel adapter (`buildKernelConnectionOptions`) forwards a custom `oauthClientId` verbatim on the U2M arm; when it is absent the napi binding applies its own default `client_id`. Whether the native binding then honors or rejects a custom id is not observable from this repo — the TypeScript layer neither hardcodes an id nor rejects one. |
62+
| `oauthClientId` + no secret | ✅ (U2M) | ✅ (U2M) | **Parity.** The kernel keys flow selection off `oauthClientSecret` presence exactly like Thrift, so `oauthClientId` + no secret routes to **U2M** (with the id forwarded) — it does **not** throw an M2M "secret required" error. |
6363
| `azureTenantId` / `useDatabricksOAuthInAzure` | ✅ | ❌ | **Thrift-only.** Kernel rejects Azure-direct (Entra) OAuth; workspace-OIDC discovery covers Azure workspaces without it. |
6464
| `persistence` (custom OAuth token store) | ✅ | ❌ | **Thrift-only.** Kernel throws; it auto-persists U2M tokens to `~/.config/databricks-sql-kernel/oauth/` and does not cache M2M. |
6565
| `authType: 'custom'` (`provider`) | ✅ | ❌ | **Thrift-only.** Kernel supports only `access-token` and `databricks-oauth`. |
@@ -105,7 +105,7 @@ column.
105105
| Option | Thrift | Kernel | Gap |
106106
| ----------------------------- | :----: | :----: | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
107107
| `preserveBigNumericPrecision` | ✅ | ✅ | DECIMAL → exact string, BIGINT → `bigint` on both. |
108-
| `enableMetricViewMetadata` | ✅ | ❌ | **Thrift-only in the connector.** Thrift auto-injects `spark.sql.thriftserver.metadata.metricview.enabled=true` (`ThriftBackend.ts`); `KernelBackend` does **not** auto-inject it. The kernel core _can_ accept the raw conf key via `session_conf`, so a caller could pass it manually in `OpenSessionRequest.configuration`. |
108+
| `enableMetricViewMetadata` | ✅ | ⚠️ | **Auto-injected for both backends** in `DBSQLClient.openSession`, which sets `spark.sql.thriftserver.metadata.metricview.enabled=true` on `request.configuration` before dispatch. `KernelBackend` folds that into `sessionOptions.sessionConf`, so the conf **does** reach the kernel session config. (`ThriftBackend.ts` performs a second, redundant injection on the Thrift path.) The kernel-side gap is that the key is a non-allowlisted session conf, so it is likely dropped by the kernel's case-insensitive allowlist (see "Session defaults") — not that it is never injected. |
109109

110110
## Session defaults (`openSession(request)`)
111111

@@ -147,14 +147,15 @@ regardless of `useKernel`.
147147

148148
### Supported on Thrift, missing / ignored on Kernel
149149

150-
1. `enableMetricViewMetadata` — no auto-injection on the kernel path.
150+
1. `enableMetricViewMetadata` — auto-injected for both backends in
151+
`DBSQLClient.openSession`, but the conf key is likely dropped by the
152+
kernel's session-conf allowlist, so it has no effect on the kernel path.
151153
2. Auth types `custom`, `token-provider`, `external-token`, `static-token`,
152154
plus `enableTokenFederation` / `federationClientId`.
153155
3. `azureTenantId` / `useDatabricksOAuthInAzure` (Azure-direct OAuth).
154156
4. `persistence` (custom OAuth token store).
155-
5. Custom `oauthClientId` on the U2M flow (and `oauthClientId` + no secret).
156-
6. SOCKS proxies.
157-
7. Per-statement `useCloudFetch`, `useLZ4Compression`,
157+
5. SOCKS proxies.
158+
6. Per-statement `useCloudFetch`, `useLZ4Compression`,
158159
`stagingAllowedLocalPath`.
159160

160161
### Supported on Kernel, no Thrift public equivalent
@@ -167,7 +168,9 @@ regardless of `useKernel`.
167168
### Behavioral divergences to watch
168169

169170
- **U2M flow selection** keys off `oauthClientSecret` presence on the kernel
170-
path but is honored differently on Thrift.
171+
path, matching Thrift: no secret ⇒ U2M, secret present ⇒ M2M. A custom
172+
`oauthClientId` (with no secret) is forwarded on the U2M arm rather than
173+
triggering an M2M "secret required" error.
171174
- **`socketTimeout: 0`** means "indefinite" on Thrift but is dropped on the
172175
kernel path (kernel default kept).
173176
- **`configuration`** is allowlist-filtered on the kernel path but forwarded

‎lib/kernel/KernelAuth.ts‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -474,21 +474,18 @@ export function buildKernelHttpOptions(options: ConnectionOptions): KernelHttpOp
474474
* a browser, listens on localhost:8030, exchanges the code, persists
475475
* to `~/.config/databricks-sql-kernel/oauth/{sha256}.json`).
476476
*
477-
* **Flow selection — DELIBERATE DIVERGENCE FROM THRIFT.** Thrift's
477+
* **Flow selection — MIRRORS THRIFT.** Thrift's
478478
* `DBSQLClient.createAuthProvider` (`DBSQLClient.ts:216`) keys off the
479479
* *secret* (`oauthClientSecret === undefined ? U2M : M2M`), so a custom
480-
* `oauthClientId` with no secret runs U2M with that id. kernel instead keys
481-
* off `oauthClientId` *presence* (id present → M2M, absent → U2M). The
482-
* trade-off: keying off the id means a caller who set an id but
483-
* typoed/forgot the secret gets the actionable M2M "secret is required"
484-
* error instead of being silently routed to U2M (which would hide their
485-
* intent). The cost is two real behavioural gaps vs Thrift:
486-
* 1. `oauthClientId` + no secret → Thrift runs U2M; kernel throws
487-
* `AuthenticationError` (M2M secret required).
488-
* 2. kernel U2M has NO custom-client-id support — the kernel hardcodes
489-
* `client_id = "databricks-cli"`, and kernel rejects any `oauthClientId`
490-
* on the U2M arm. Thrift U2M honours a custom `clientId`.
491-
* Both are documented limitations of the M0 kernel OAuth surface, not bugs.
480+
* `oauthClientId` with no secret runs U2M with that id. This adapter keys
481+
* off the same signal: `oauthClientSecret === undefined` ⇒ U2M, else M2M
482+
* (see `buildKernelConnectionOptions` below). A custom `oauthClientId` is
483+
* forwarded verbatim on the U2M arm; when absent the napi binding applies
484+
* its own default `client_id`. The adapter therefore does NOT throw an M2M
485+
* "secret required" error for `oauthClientId` + no secret, and does NOT
486+
* reject a custom `oauthClientId` on U2M — those decisions, if the native
487+
* binding makes them, happen below the TypeScript layer and are not
488+
* observable from this repo.
492489
*
493490
* Out of scope on the OAuth paths (rejected with a clear error):
494491
* - `azureTenantId` / `useDatabricksOAuthInAzure` → Microsoft Entra

‎lib/kernel/KernelBackend.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ export interface KernelBackendOptions {
5757
* (slash-prepended httpPath, AuthenticationError on missing token or
5858
* blank OAuth credentials, HiveDriverError on unsupported authType /
5959
* Azure-direct / ambiguous credential combinations). M2M and U2M
60-
* routing key off `oauthClientId` presence; see KernelAuth.ts.
60+
* routing key off `oauthClientSecret` presence (mirroring Thrift); see
61+
* KernelAuth.ts.
6162
*
6263
* **Why we don't use IClientContext's connectionProvider here:** that
6364
* provider is the Thrift HTTP transport. The kernel owns its own

0 commit comments

Comments
 (0)