Skip to content

Conversation

@jackyhu-db
Copy link
Contributor

@jackyhu-db jackyhu-db commented Jul 2, 2025

PR Description

Description

This PR set the SqlState of the timeout exception raised in Databricks RetryHttpHandler to align SqlState used in same exception in other Databricks drivers including JDBC.

Changes

  • Set the SqlState of the timeout exception raised in Databricks RetryHttpHandler
  • Fixed a bug in the unit test RetryHttpHandlerTest

Motivation

Fix the failure in unit test RetryHttpHandlerTest and align the error message with other Databricks drivers.

Testing

  • Unt test RetryHttpHandlerTest
  • End-to-end test

@github-actions github-actions bot added this to the ADBC Libraries 20 milestone Jul 2, 2025
Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Can you set the SqlState on the exception explicitly? (This may or may not remove the need to also add it to the message.)


throw new DatabricksException(
lastErrorMessage ?? "Service temporarily unavailable and retry timeout exceeded",
"[SQLState: 08001]" + (lastErrorMessage ?? "Service temporarily unavailable and retry timeout exceeded"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a separate field on DatabricksException for SqlState. It's not in the constructor, but you can do
throw new DatabricksException(...).SetSqlState("08001");

I don't know if that would also make the message come out correctly where you want it to, but having the SqlState as a property without having to parse it out of the message is definitely a good thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. It makes sense and I uploaded the patch to fix it.

@jackyhu-db jackyhu-db changed the title fix(csharp/src/Drivers/Databricks): Fix the error message in RetryHttpHandler fix(csharp/src/Drivers/Databricks): Set the SqlState of the exception in RetryHttpHandler Jul 3, 2025
@CurtHagenlocher CurtHagenlocher merged commit 031654f into apache:main Jul 3, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants