Skip to content

Commit 4a7198d

Browse files
ai: apply changes for #506 (2 review threads)
Addresses: - #3876121332 at tests/unit/DBSQLClient.test.ts:966 - #3876121337 at lib/kernel/KernelAuth.ts:685 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 27e4f8e commit 4a7198d

2 files changed

Lines changed: 15 additions & 2 deletions

File tree

lib/kernel/KernelAuth.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -701,10 +701,15 @@ export function buildKernelTelemetryOptions(
701701
if (Number.isFinite(config.telemetryCloseTimeoutMs)) {
702702
telemetry.telemetryCloseFlushTimeoutMs = config.telemetryCloseTimeoutMs;
703703
}
704-
if (Number.isFinite(config.telemetryCircuitBreakerThreshold)) {
704+
// The breaker is forced on above, and the napi contract requires threshold/timeout
705+
// to be strictly positive when it is enabled. A caller-supplied `0` (or negative)
706+
// would otherwise be forwarded verbatim and surface as a hard kernel `openSession`
707+
// rejection, so treat any non-positive value as a misconfiguration and fall back to
708+
// the kernel defaults (5 / 60000) instead.
709+
if (Number.isFinite(config.telemetryCircuitBreakerThreshold) && config.telemetryCircuitBreakerThreshold! > 0) {
705710
telemetry.telemetryCircuitBreakerThreshold = config.telemetryCircuitBreakerThreshold;
706711
}
707-
if (Number.isFinite(config.telemetryCircuitBreakerTimeout)) {
712+
if (Number.isFinite(config.telemetryCircuitBreakerTimeout) && config.telemetryCircuitBreakerTimeout! > 0) {
708713
telemetry.telemetryCircuitBreakerTimeoutMs = config.telemetryCircuitBreakerTimeout;
709714
}
710715

tests/unit/DBSQLClient.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import DBSQLClient, { ThriftLibrary } from '../../lib/DBSQLClient';
55
import DBSQLSession from '../../lib/DBSQLSession';
66
import ThriftBackend from '../../lib/thrift-backend/ThriftBackend';
77
import KernelBackend from '../../lib/kernel/KernelBackend';
8+
import * as KernelNativeLoader from '../../lib/kernel/KernelNativeLoader';
89

910
import PlainHttpAuthentication from '../../lib/connection/auth/PlainHttpAuthentication';
1011
import DatabricksOAuth from '../../lib/connection/auth/DatabricksOAuth';
@@ -963,6 +964,13 @@ describe('DBSQLClient telemetry paths', () => {
963964
delete process.env.DATABRICKS_TELEMETRY_DISABLED;
964965
const client = new DBSQLClient();
965966
const initStub = sinon.stub(client as any, 'initializeTelemetry').resolves();
967+
// DBSQLClient constructs `new KernelBackend({ context: this })` without
968+
// injecting a `nativeBinding`, so the KernelBackend constructor calls
969+
// `getKernelNative()`, which throws where the native `.node` artifact
970+
// isn't built (e.g. CI). Stub the loader so construction succeeds and the
971+
// assertion below tests the `!useKernel` telemetry gate rather than the
972+
// presence of a built kernel artifact.
973+
sinon.stub(KernelNativeLoader, 'getKernelNative').returns({} as any);
966974
sinon.stub(KernelBackend.prototype, 'connect').resolves();
967975
sinon.stub(KernelBackend.prototype, 'close').resolves();
968976

0 commit comments

Comments
 (0)