Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Enhance SEA HTTP Client#618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
varun-edachali-dbx merged 453 commits intomainfromsea-http-client
Aug 4, 2025
Merged

Enhance SEA HTTP Client#618

varun-edachali-dbx merged 453 commits intomainfromsea-http-client
Aug 4, 2025

Conversation

@varun-edachali-dbx
Copy link
Contributor

@varun-edachali-dbxvarun-edachali-dbx commentedJun 27, 2025•
edited
Loading

What type of PR is this?

  • Feature

Description

Enhance the current, simplified SEA Http Client by introducing retries. Most of the auth related functionality is pulled from the Thrift backend. All of the retry-related tests are expected to pass since they represent user-facing functionality (in terms of exceptions to be thrown in particular cases).

We continue using a low-level connection pool instead of using the higher levelrequests module because the latter does not have first-class support for passing a password to a client certificate, which we want to support. There are workaroundsinvolving subclassing theHttpAdapter and invokinginit_poolmanager, but the docstring of the method states that it should not be invoked from client code, so we do not consider it a long term solution.

The exceptions thrown by the SEA and Thrift backends are not completely the same (yet), because this PR passesNone to the context and session id of theRequestError object. This misparity will be logged and addressed later. Here, the context is expected to have the following keys as per the existing class definition:

class RequestError(OperationalError):    """Thrown if there was a error during request to the server.    Its context will have the following keys:    "method": The RPC method name that failed    "session-id": The Thrift session guid    "query-id": The Thrift query guid (if available)    "http-code": HTTP response code to RPC request (if available)    "error-message": Error message from the HTTP headers (if available)    "original-exception": The Python level original exception    "no-retry-reason": Why the request wasn't retried (if available)    "bounded-retry-delay": The maximum amount of time an error will be retried before giving up    "attempt": current retry number / maximum number of retries    "elapsed-seconds": time that has elapsed since first attempting the RPC request    """    pass

How?

In_make_request(), the client sets up the retry context before making the HTTP request:

  • Determines the operation type (EXECUTE_STATEMENT, GET_OPERATION_STATUS, etc.) based on the API path and HTTP method
  • Sets the command type on the DatabricksRetryPolicy - this influences retry decisions (e.g., EXECUTE_STATEMENT is only retried for 429/503 status codes)
  • Starts the retry timer to track total request duration across all retry attempts
  • Makes the HTTP request through urllib3 with the retry policy attached

The retry policy then handles all retry logic automatically - determining when to retry based on status codes and command types, calculating backoff delays, and enforcing attempt/duration limits. If retries are exhausted, urllib3 raises a MaxRetryError which gets caught and wrapped in a RequestError.

How is this tested?

  • Unit tests
  • E2E Tests
  • Manually
  • N/A

Related Tickets & Documents

Design Doc

varun-edachali-dbxand others added30 commitsJune 18, 2025 05:46
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
* [squash from exec-sea] bring over execution phase changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess testSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add docstringSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remvoe exec func in sea backendSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess filesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess modelsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess sea backend testsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* cleanupSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* re-introduce get_schema_descSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove SeaResultSetSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* clean imports and attributesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* pass CommandId to ExecRespSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove changes in typesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add back essential types (ExecResponse, from_sea_state)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix fetch typesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* excess importsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* reduce diff by maintaining logsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix int test typesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* [squashed from exec-sea] init execution funcSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove irrelevant changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove ResultSetFilter functionalitySigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove more irrelevant changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove more irrelevant changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* even more irrelevant changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove sea response as init optionSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* exec test example scriptsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* [squashed from sea-exec] merge sea stuffsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess removed docstringSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess changes in backendSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess importsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove accidentally removed _get_schema_descSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove unnecessary init with sea_response testsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* rmeove unnecessary changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* improved models and filters from cloudfetch-sea branchSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* filters stuff (align with JDBC)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* backend from cloudfetch-seaSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove filtering, metadata opsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* raise NotImplementedErrror for metadata opsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* change to valid table nameSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove un-necessary changescovered by#588Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* simplify test moduleSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* logging -> debug levelSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* change table name in logSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove un-necessary changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove un-necessary backend cahngesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove un-needed GetChunksResponseSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove un-needed GetChunksResponseonly relevant in Fetch phaseSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* reduce code duplication in response parsingSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* reduce code duplicationSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* more clear docstringsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* introduce strongly typed ChunkInfoSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove is_volume_operation from responseSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add is_volume_op and more ResultData fieldsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add test scriptsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* Revert "Merge branch 'exec-models-sea' into exec-phase-sea"This reverts commitbe1997e, reversingchanges made to37813ba.* change logging levelSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove un-necessary changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove excess changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove _get_schema_bytes (for now)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* redundant commentsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove fetch phase methodsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* reduce code repetititon + introduce gaps after multi line pydocsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove unused importsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* move description extraction to helper funcSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add more unit testsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* streamline unit testsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* test getting the list of allowed configurationsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* reduce diffSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* reduce diffSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* house constants in enums for readability and immutabilitySigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add note on hybrid dispositionSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove redundant note on arrow_schema_bytesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove invalid importSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add strong typing for manifest in _extract_descriptionSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove un-necessary column skippingSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove parsing in backendSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix: convert sea statement id to CommandId typeSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* make polling interval a separate constantSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* align state checking with Thrift implementationSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* update unit tests according to changesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add unit tests for added methodsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add spec to description extraction docstring, add strong typing to paramsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add strong typing for backend parameters argSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>---------Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
…nd forward refs, remove some unused importsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Co-authored-by: jayant <167047871+jayantsing-db@users.noreply.github.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
…ionsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
This reverts commitdabba55, reversingchanges made todd7dc6a.Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Copy link
Contributor

@jayantsing-dbjayantsing-db left a comment

Choose a reason for hiding this comment

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

Few comments

@jayantsing-db
Copy link
Contributor

The exceptions thrown by the SEA and Thrift backends are not completely the same (yet), because this PR passes None to the context and session id of the RequestError object. This misparity will be logged and addressed later.

Could you add details on this? Is there a follow-up PR? what is the context and session id?

varun-edachali-dbx reacted with thumbs up emoji

Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Copy link
Contributor

@jayantsing-dbjayantsing-db left a comment

Choose a reason for hiding this comment

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

LGTM

@databricksdatabricks deleted a comment fromgithub-actionsbotAug 4, 2025
@varun-edachali-dbxvarun-edachali-dbx merged commit3b0c882 intomainAug 4, 2025
22 of 23 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jayantsing-dbjayantsing-dbjayantsing-db approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@varun-edachali-dbx@jayantsing-db@saishreeeee
[8]ページ先頭
©2009-2025 Movatter.jp