[vuanhphung]

Description

Remove automatic Thrift fallback when SEA session creation returns HTTP 307. The driver keeps the selected protocol and surfaces the existing CONNECTION_ERROR (1001), preserving the SDK error as its cause.

The fallback was added in #676 as a server-controlled kill switch to protect SEA capacity during the JDBC GA rollout. SEA returns 307 on session creation when the User-Agent matches a known driver but the workspace's databricks.sqlexecapi.allowed_driver_traffic SAFE flag excludes that driver. The response includes Location: /sql/1.0/warehouses, asking the driver to establish a Thrift session.

DatabricksJDBCDriverOSS should now be allowlisted; the server's code default already includes it. Live SAFE overrides have not been verified. An override excluding JDBC would cause a connection failure after this change instead of falling back to Thrift.

Testing

257 focused tests passed across SDK behavior, session handling, client selection, and Reyden recovery. New tests exercise HTTP 307 through the real SDK for automatic SEA routing and explicit UseThriftClient=0. Spotless and git diff --check passed.

Telemetry Errors

• Not applicable — this PR does not add or change a telemetry-visible error.
• Applicable — the error uses DatabricksDriverErrorCode where appropriate, and any new code is uniquely numbered and tested.
• Applicable — its driver/server/user classification is linked, or maintainer help is requested because the author cannot access the classification.

HTTP 307 now emits CONNECTION_ERROR (1001) at ERROR instead of TEMPORARY_REDIRECT_EXCEPTION (1031) at TRACE; 1031 remains reserved. Maintainers: please confirm/update the driver/server/user classification for this case in the OSS JDBC telemetry dashboard and automation taxonomy.

Additional Notes to the Reviewer

A September 28–October 28 telemetry check found no events with the old redirect error name or message. TRACE events are filtered by the default DEBUG setting, so this does not prove the fallback was unused.

[peco-review-bot]

Verdict: 1 Low

Looks good: the fallback removal is clean, no references to the deleted exception or constant are left, and the new real-SDK tests cover both automatic and explicit SEA routing. The only finding is a low-severity suggestion to make the HTTP 307 error message tell users what to do.

Other findings

• 🔵 Low — An HTTP 307 on CreateSession now goes through the generic buildErrorMessage path. The user sees something like Error while establishing a connection in databricks: Redirect to Thrift Client (HTTP 307), logged at ERROR, with no hint about what to do next. Connections that used to fall back to Thrift without any visible error, especially ones on automatic SEA routing where the user never chose a protocol, will now fail at open().

The 429 branch just above has its own message. A 307-specific message telling the user to set UseThriftClient=1 (or to contact support) would make this behavior change much easier to diagnose. The changelog entry could say the same. This is optional; the current behavior is correct as written.

[peco-review-bot]

Verdict: 1 Low

Looks good. The fallback removal is clean: I found no other references to the deleted exception or constant, and a 307 now surfaces as CONNECTION_ERROR (1001) with the SDK error kept as its cause, which the real-SDK tests check. I have one low-severity suggestion: the error message users see on a 307 could say what to do next.

Other findings

• 🔵 Low — With the special case removed, a 307 now goes through the generic buildErrorMessage. The user sees something like Error while establishing a connection in databricks: Redirect to Thrift Client (HTTP 307). That text is misleading because the driver no longer redirects, and it doesn't tell the user what to do. The PR description says the expected cause is a SAFE override that excludes JDBC from SEA, so these users would be stuck.

Consider keeping a small e.getStatusCode() == 307 branch that still throws CONNECTION_ERROR with the SDK error as cause, but adds a hint: SEA rejected this driver for the workspace, and setting UseThriftClient=1 connects over Thrift. Then the 429 branch would no longer be the only status with a tailored message. The existing tests' contains("HTTP 307") assertions would still pass.

[jay-xiao446]

lgtm

[peco-review-bot]

Verdict: 1 Low

Looks good. The fallback is fully removed with no dangling references, a 307 now surfaces as CONNECTION_ERROR with the SDK cause preserved, and the real-SDK tests check that no Thrift client is ever built. One low note: now that the fallback is gone, the error message for this new hard-failure case could suggest a workaround.

Other findings

• 🔵 Low — A 307 from SEA CreateSession now goes through the generic buildErrorMessage(e) path. The user sees Error while establishing a connection in databricks: Redirect to Thrift Client (HTTP 307). The PR description says a SAFE override that excludes JDBC will now cause a hard connection failure. For anyone hit by that, the server's wording ("Redirect to Thrift Client") suggests the driver will redirect, but it no longer does, and the message doesn't tell them how to fix it.

Consider a small dedicated branch for e.getStatusCode() == 307, like the existing 429 branch. It would still throw CONNECTION_ERROR with e as the cause, but say that the server asked for a Thrift session and that UseThriftClient=1 works around it. That gives the new failure mode a clear workaround without bringing the fallback back. The existing tests already check that the message contains HTTP 307, so you'd only need to adjust them a little.

[peco-review-bot]

Verdict: 1 Low

Looks good. The removal is clean, with no leftover references to the deleted exception or constant, and the tests run the 307 response through the real SDK for both the automatic and explicit SEA paths. One low-severity concern: a user who hits a 307 now gets an error message that doesn't tell them what to do.

Other findings

• 🔵 Low — With the 307 branch gone, a SAFE-excluded workspace now gets the generic error Error while establishing a connection in databricks: Redirect to Thrift Client (HTTP 307) (your new test asserts this exact text). The server is telling the driver to use Thrift, but the driver no longer does that, so the message gives the user nothing to act on. The PR description says this case now fails the connection where it used to fall back silently. Consider an explicit SC_TEMPORARY_REDIRECT branch next to the 429 handling. It would keep CONNECTION_ERROR and the cause, and add a hint such as "SEA is not enabled for this driver on this workspace; set UseThriftClient=1 to connect via Thrift". It could also log at WARN, which makes this rollout risk easier to diagnose from customer logs.