[vuanhphung]

DBSQLClient.getConnectionOptions() hardcoded https: true, so apps that reach Databricks through a trusted local reverse proxy (app --HTTP--> proxy --HTTPS--> Databricks) had to monkey-patch that private method, even though HttpConnection already supports plain HTTP.

This adds a public opt-in, https?: boolean, to ConnectionOptions. It matches the internal IConnectionOptions.https it maps to. HTTPS stays the default, and only an explicit false turns it off. With https: false the default port is 80, and clientCert/clientKey are rejected instead of being silently dropped (the kernel rejects that combination too). The kernel binding keeps an explicit http:// host, so the kernel backend gets http://host[:port]. That also keeps Thrift's Reyden fallback to the kernel on the same transport. On Thrift the scope is deliberately limited to the SQL connection: OAuth, token federation, telemetry and feature-flag requests still use HTTPS. CloudFetch presigned URLs are untouched. CONNECTION_PARAMETERS.md documents this and warns that plain HTTP is only for trusted local or private links.

Verification: new unit tests cover the option mapping and run a real open/close round trip against a local plain-HTTP Thrift server. Without the fix, that round trip fails with a TLS wrong version number error. A new e2e spec runs SELECT 1 on a live warehouse over https: false through a local reverse proxy on both backends. The existing proxy e2e test now uses the public option instead of overriding the private method. On the default path, a live 200k-row query on both backends confirmed Thrift still uses an https.Agent on port 443 and the kernel host is passed through unchanged. npm test (1537 passing), lint, type-check and build pass. Performance isn't affected: the change runs once per connect() while building options, and no per-request or per-row code changed.

Fixes #503

This pull request and its description were written by Isaac.

[peco-review-bot]

Verdict: 1 Low

Looks good overall, with one low-severity edge case. On the kernel path with https: false, setting a port both in host and in port produces a doubled port in the URL. The mapping is correct otherwise: Thrift defaults, the kernel http:// host and rejecting mTLS over plain HTTP all work, and mTLS is rejected on both backends because the kernel path also calls createConnectionProvider.

[peco-review-bot]

Verdict: 1 Low

Looks good, with one low-severity edge case in how the kernel builds the host URL for bare IPv6 hosts. The option mapping, the mTLS rejection, the docs and the tests (including a real plain-HTTP Thrift round trip) match the intended behaviour. The Thrift-only mTLS check is fine because the kernel enforces it itself.

[peco-review-bot]

Verdict: 1 Low

Looks good: one low-severity concern about a trailing slash in the kernel http:// host. The https option maps cleanly onto HttpConnection's existing plain-HTTP support. The defaults stay HTTPS-only, mTLS is rejected up front, and both backends have unit and e2e coverage.

[peco-review-bot]

✅ No issues identified by the review bot.

[vuanhphung]

@bot https: false also changes the agent reused by CloudFetch and OAuth to http.Agent. Their HTTPS requests then fail with ERR_INVALID_PROTOCOL, even though their URLs remain unchanged. Could we select the agent by each request’s protocol and cover HTTP SQL transport alongside HTTPS CloudFetch/OAuth?

[peco-engineer-bot]

Pushed 2fe61fd to feat/issue-503-http-transport addressing your request.

Started from 6e5c655. Fixed: getAgent(url?) now picks the agent from the URL's protocol, while https only sets the SQL agent. CloudFetch, staging, telemetry and feature flags pass their URLs; OAuth chooses per request. Proxy behavior is unchanged. New tests cover HTTP SQL with HTTPS CloudFetch and OAuth; without the fix they hit ERR_INVALID_PROTOCOL. The PR's mock server answered 500 here (generated code needs Q), so its handlers are now callback-style. npm test (1545), type-check, prettier pass. Lint can't run: airbnb-base is missing. Human needed: delete the empty tests/unit/tmp_debug.test.ts (I can't remove files). E2E not run.

[peco-review-bot]

Verdict: 1 Low

Looks good, with one low-severity cleanup: a stray empty debug test file. getAgent now picks the agent from each request's protocol, and every caller in lib/ passes its URL except the Thrift connection, which correctly uses the SQL-transport default. That keeps CloudFetch, OAuth, telemetry and feature-flag requests on HTTPS when https: false is set, and the new unit tests cover this.

Other findings

• 🔵 Low — This PR adds an empty file, tests/unit/tmp_debug.test.ts (blob e69de29b is the empty blob). It looks like a leftover from local debugging. It has no tests, but it matches the unit test glob, so mocha loads it on every run and it stays in the tree as noise. Please delete it before merging.

[vuanhphung]

@bot Kernel host already supports full URLs. With { host: 'http://localhost:3015', https: false }, this returns http://http://localhost:3015; an https:// host gets double-prefixed too. Adding the new option therefore breaks these configurations. Please normalize the existing scheme before applying HTTP and checking for an embedded port, and add tests for both URL forms.

[peco-engineer-bot]

Pushed db27afe to feat/issue-503-http-transport addressing your request.

Started from 4657692. Fixed in buildKernelHostName (lib/kernel/KernelAuth.ts): with https: false it now strips any existing http:// or https:// scheme (case-insensitive) first, then the trailing /, then checks for an embedded port and applies http://. So 'http://localhost:3015' and 'https://localhost:3015' both become 'http://localhost:3015', with no doubled port. Hosts are unchanged when https isn't false. Added tests in tests/unit/kernel/connectionOptions.test.ts for both URL forms, with and without port, upper case, trailing slash and IPv6. Build, type-check, prettier and npm test pass (1684). npm run lint failed: ESLint can't find the airbnb-base config in this runner. Kernel e2e not run (no warehouse or native build).

[peco-review-bot]

Verdict: 1 Low

Looks good, with one low-severity issue: a stray empty debug test file was committed. The agent is now picked by each request's protocol, so CloudFetch, OAuth, telemetry, feature-flag and staging requests keep HTTPS while the SQL transport uses plain HTTP. That change is consistent across all getAgent callers and has unit-test coverage.

Other findings

• 🔵 Low — This PR adds tests/unit/tmp_debug.test.ts as an empty file. Going by its name, it's a leftover debug scratch file. It has no tests, but mocha's unit glob still picks it up, and it leaves a stray tmp_ file in the repo. Please remove it from the PR.