mirror of
https://github.com/Kaelio/ktx.git
synced 2026-07-25 12:01:03 +02:00
fix(cli): honor KTX_SQL_ANALYSIS_URL/KTX_DAEMON_URL in ktx sql and MCP
ktx ingest resolved the SQL-analysis port via resolveKtxCliSqlAnalysis, which honors the external-daemon env overrides, but `ktx sql` and the MCP server built their port by calling createManagedDaemonSqlAnalysisPort directly. On a uv-less host with an external daemon and the override set, ingest worked while `ktx sql` and MCP sql_execution ignored the override and tried to install the managed Python runtime — contradicting the documented contract and runtime-requirements, which already skip the managed runtime when those vars are set. Collapse the three call sites onto one env-override-aware resolver: add externalDaemonSqlAnalysisBaseUrl + resolveSqlAnalysisPort in managed-python-http.ts and route sql.ts, mcp-server-factory.ts, and resolveKtxCliSqlAnalysis through it. createManagedDaemonSqlAnalysisPort becomes @internal (test-only export).
This commit is contained in:
parent
663eaff940
commit
7e5551906e
6 changed files with 130 additions and 14 deletions
|
|
@ -29,8 +29,9 @@ import { createHttpSqlAnalysisPort } from './context/sql-analysis/http-sql-analy
|
|||
import type { SqlAnalysisPort } from './context/sql-analysis/ports.js';
|
||||
import {
|
||||
createManagedDaemonLookerTableIdentifierParser,
|
||||
createManagedDaemonSqlAnalysisPort,
|
||||
externalDaemonSqlAnalysisBaseUrl,
|
||||
managedDaemonDatabaseIntrospectionOptions,
|
||||
resolveSqlAnalysisPort,
|
||||
type ManagedPythonDaemonHttpOptions,
|
||||
} from './managed-python-http.js';
|
||||
import type { KtxOperationalLogger } from './io/logger.js';
|
||||
|
|
@ -78,16 +79,12 @@ export function resolveKtxCliSqlAnalysis(options: KtxCliLocalIngestAdaptersOptio
|
|||
if (options.sqlAnalysisUrl) {
|
||||
return createHttpSqlAnalysisPort({ baseUrl: options.sqlAnalysisUrl });
|
||||
}
|
||||
if (process.env.KTX_SQL_ANALYSIS_URL) {
|
||||
return createHttpSqlAnalysisPort({ baseUrl: process.env.KTX_SQL_ANALYSIS_URL });
|
||||
}
|
||||
if (process.env.KTX_DAEMON_URL) {
|
||||
return createHttpSqlAnalysisPort({ baseUrl: process.env.KTX_DAEMON_URL });
|
||||
}
|
||||
if (options.managedDaemon) {
|
||||
return createManagedDaemonSqlAnalysisPort(options.managedDaemon);
|
||||
return resolveSqlAnalysisPort(options.managedDaemon);
|
||||
}
|
||||
return createHttpSqlAnalysisPort({ baseUrl: 'http://127.0.0.1:8765' });
|
||||
return createHttpSqlAnalysisPort({
|
||||
baseUrl: externalDaemonSqlAnalysisBaseUrl(process.env) ?? 'http://127.0.0.1:8765',
|
||||
});
|
||||
}
|
||||
|
||||
function createKtxCliLiveDatabaseIntrospection(
|
||||
|
|
|
|||
|
|
@ -178,6 +178,7 @@ export function createManagedDaemonLookerTableIdentifierParser(
|
|||
});
|
||||
}
|
||||
|
||||
/** @internal */
|
||||
export function createManagedDaemonSqlAnalysisPort(options: ManagedPythonDaemonHttpOptions): SqlAnalysisPort {
|
||||
return createHttpSqlAnalysisPort({
|
||||
baseUrl: 'http://127.0.0.1:0',
|
||||
|
|
@ -185,6 +186,31 @@ export function createManagedDaemonSqlAnalysisPort(options: ManagedPythonDaemonH
|
|||
});
|
||||
}
|
||||
|
||||
export function externalDaemonSqlAnalysisBaseUrl(
|
||||
env: NodeJS.ProcessEnv | Record<string, string | undefined>,
|
||||
): string | undefined {
|
||||
const sqlAnalysisUrl = env.KTX_SQL_ANALYSIS_URL?.trim();
|
||||
if (sqlAnalysisUrl) {
|
||||
return sqlAnalysisUrl;
|
||||
}
|
||||
const daemonUrl = env.KTX_DAEMON_URL?.trim();
|
||||
if (daemonUrl) {
|
||||
return daemonUrl;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
export function resolveSqlAnalysisPort(
|
||||
managedDaemon: ManagedPythonDaemonHttpOptions,
|
||||
env: NodeJS.ProcessEnv | Record<string, string | undefined> = process.env,
|
||||
): SqlAnalysisPort {
|
||||
const externalBaseUrl = externalDaemonSqlAnalysisBaseUrl(env);
|
||||
if (externalBaseUrl) {
|
||||
return createHttpSqlAnalysisPort({ baseUrl: externalBaseUrl });
|
||||
}
|
||||
return createManagedDaemonSqlAnalysisPort(managedDaemon);
|
||||
}
|
||||
|
||||
export function managedDaemonDatabaseIntrospectionOptions(
|
||||
options: ManagedPythonDaemonHttpOptions,
|
||||
): Pick<DaemonLiveDatabaseIntrospectionOptions, 'requestJson'> {
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ import { resolveProjectEmbeddingProvider } from './embedding-resolution.js';
|
|||
import { createKtxCliIngestQueryExecutor } from './ingest-query-executor.js';
|
||||
import { createKtxCliScanConnector } from './local-scan-connectors.js';
|
||||
import { createManagedPythonSemanticLayerComputePort } from './managed-python-command.js';
|
||||
import { createManagedDaemonSqlAnalysisPort } from './managed-python-http.js';
|
||||
import { resolveSqlAnalysisPort } from './managed-python-http.js';
|
||||
|
||||
function noopMcpIo(): KtxCliIo {
|
||||
return {
|
||||
|
|
@ -31,7 +31,7 @@ export async function createKtxMcpServerFactory(input: {
|
|||
installPolicy: 'auto',
|
||||
io,
|
||||
});
|
||||
const sqlAnalysis = createManagedDaemonSqlAnalysisPort({
|
||||
const sqlAnalysis = resolveSqlAnalysisPort({
|
||||
cliVersion: input.cliVersion,
|
||||
projectDir: input.projectDir,
|
||||
installPolicy: 'auto',
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import type { SqlAnalysisDialect, SqlAnalysisPort } from './context/sql-analysis
|
|||
import type { KtxCliIo } from './cli-runtime.js';
|
||||
import { type KtxOutputMode, resolveOutputMode } from './io/mode.js';
|
||||
import { createKtxCliScanConnector } from './local-scan-connectors.js';
|
||||
import { createManagedDaemonSqlAnalysisPort } from './managed-python-http.js';
|
||||
import { resolveSqlAnalysisPort } from './managed-python-http.js';
|
||||
import { profileMark } from './startup-profile.js';
|
||||
import { isDemoConnection } from './telemetry/demo-detect.js';
|
||||
import { emitTelemetryEvent, reportException } from './telemetry/index.js';
|
||||
|
|
@ -156,7 +156,7 @@ export async function runKtxSql(args: KtxSqlArgs, io: KtxCliIo = process, deps:
|
|||
const createSqlAnalysis =
|
||||
deps.createSqlAnalysis ??
|
||||
(() =>
|
||||
createManagedDaemonSqlAnalysisPort({
|
||||
resolveSqlAnalysisPort({
|
||||
cliVersion: args.cliVersion,
|
||||
projectDir: args.projectDir,
|
||||
installPolicy: 'auto',
|
||||
|
|
|
|||
|
|
@ -1,10 +1,14 @@
|
|||
import { createServer } from 'node:http';
|
||||
import type { AddressInfo } from 'node:net';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import {
|
||||
createManagedDaemonHttpJsonRunner,
|
||||
createManagedDaemonLookerTableIdentifierParser,
|
||||
createManagedDaemonSqlAnalysisPort,
|
||||
createManagedPythonDaemonBaseUrlResolver,
|
||||
externalDaemonSqlAnalysisBaseUrl,
|
||||
managedDaemonDatabaseIntrospectionOptions,
|
||||
resolveSqlAnalysisPort,
|
||||
} from '../src/managed-python-http.js';
|
||||
|
||||
function io() {
|
||||
|
|
@ -18,6 +22,25 @@ function io() {
|
|||
};
|
||||
}
|
||||
|
||||
async function startLoopbackDaemon() {
|
||||
const paths: string[] = [];
|
||||
const server = createServer((req, res) => {
|
||||
paths.push(req.url ?? '');
|
||||
req.on('data', () => {});
|
||||
req.on('end', () => {
|
||||
res.writeHead(200, { 'content-type': 'application/json' });
|
||||
res.end(JSON.stringify({ ok: true }));
|
||||
});
|
||||
});
|
||||
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
|
||||
const { port } = server.address() as AddressInfo;
|
||||
return {
|
||||
baseUrl: `http://127.0.0.1:${port}`,
|
||||
paths,
|
||||
close: () => new Promise<void>((resolve, reject) => server.close((error) => (error ? reject(error) : resolve()))),
|
||||
};
|
||||
}
|
||||
|
||||
describe('createManagedPythonDaemonBaseUrlResolver', () => {
|
||||
it('ensures the core runtime, starts the daemon, reports the URL, and caches the result', async () => {
|
||||
const testIo = io();
|
||||
|
|
@ -203,3 +226,73 @@ describe('ktx daemon ingest ports', () => {
|
|||
expect(requestJson).toHaveBeenCalledWith('/database/introspect', { connection_id: 'warehouse' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('externalDaemonSqlAnalysisBaseUrl', () => {
|
||||
it('prefers KTX_SQL_ANALYSIS_URL over KTX_DAEMON_URL', () => {
|
||||
expect(
|
||||
externalDaemonSqlAnalysisBaseUrl({
|
||||
KTX_SQL_ANALYSIS_URL: 'http://analysis.example',
|
||||
KTX_DAEMON_URL: 'http://daemon.example',
|
||||
}),
|
||||
).toBe('http://analysis.example');
|
||||
});
|
||||
|
||||
it('falls back to KTX_DAEMON_URL when only it is set', () => {
|
||||
expect(externalDaemonSqlAnalysisBaseUrl({ KTX_DAEMON_URL: 'http://daemon.example' })).toBe('http://daemon.example');
|
||||
});
|
||||
|
||||
it('trims surrounding whitespace and ignores whitespace-only overrides', () => {
|
||||
expect(externalDaemonSqlAnalysisBaseUrl({ KTX_SQL_ANALYSIS_URL: ' http://analysis.example ' })).toBe(
|
||||
'http://analysis.example',
|
||||
);
|
||||
expect(externalDaemonSqlAnalysisBaseUrl({ KTX_SQL_ANALYSIS_URL: ' ', KTX_DAEMON_URL: '' })).toBeUndefined();
|
||||
expect(externalDaemonSqlAnalysisBaseUrl({})).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveSqlAnalysisPort', () => {
|
||||
it('uses the managed daemon when no external override is set', async () => {
|
||||
const daemon = await startLoopbackDaemon();
|
||||
const testIo = io();
|
||||
const ensureRuntime = vi.fn(async () => ({ layout: {} as never, manifest: {} as never }));
|
||||
const startDaemon = vi.fn(async () => ({
|
||||
status: 'started' as const,
|
||||
layout: {} as never,
|
||||
state: { pid: 1234 } as never,
|
||||
baseUrl: daemon.baseUrl,
|
||||
}));
|
||||
try {
|
||||
const port = resolveSqlAnalysisPort(
|
||||
{ cliVersion: '0.2.0', projectDir: '/work/proj', installPolicy: 'auto', io: testIo.io, ensureRuntime, startDaemon },
|
||||
{},
|
||||
);
|
||||
await expect(port.validateReadOnly('select 1', 'postgres')).resolves.toEqual({ ok: true });
|
||||
expect(ensureRuntime).toHaveBeenCalledTimes(1);
|
||||
expect(startDaemon).toHaveBeenCalledTimes(1);
|
||||
expect(daemon.paths).toContain('/sql/validate-read-only');
|
||||
} finally {
|
||||
await daemon.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('routes to the external daemon and skips the managed runtime when KTX_SQL_ANALYSIS_URL is set', async () => {
|
||||
const daemon = await startLoopbackDaemon();
|
||||
const testIo = io();
|
||||
const ensureRuntime = vi.fn(async () => ({ layout: {} as never, manifest: {} as never }));
|
||||
const startDaemon = vi.fn(async () => {
|
||||
throw new Error('managed runtime must not start when an external daemon URL is set');
|
||||
});
|
||||
try {
|
||||
const port = resolveSqlAnalysisPort(
|
||||
{ cliVersion: '0.2.0', projectDir: '/work/proj', installPolicy: 'auto', io: testIo.io, ensureRuntime, startDaemon },
|
||||
{ KTX_SQL_ANALYSIS_URL: daemon.baseUrl },
|
||||
);
|
||||
await expect(port.validateReadOnly('select 1', 'postgres')).resolves.toEqual({ ok: true });
|
||||
expect(ensureRuntime).not.toHaveBeenCalled();
|
||||
expect(startDaemon).not.toHaveBeenCalled();
|
||||
expect(daemon.paths).toContain('/sql/validate-read-only');
|
||||
} finally {
|
||||
await daemon.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -66,7 +66,7 @@ vi.mock('../src/managed-python-command.js', () => ({
|
|||
}));
|
||||
|
||||
vi.mock('../src/managed-python-http.js', () => ({
|
||||
createManagedDaemonSqlAnalysisPort: vi.fn(() => mocks.sqlAnalysis),
|
||||
resolveSqlAnalysisPort: vi.fn(() => mocks.sqlAnalysis),
|
||||
}));
|
||||
|
||||
const project = {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue