-
Notifications
You must be signed in to change notification settings - Fork 120
Refactor codebase to use a unified http client #673
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the codebase to use a unified HTTP client across all HTTP operations in the Databricks SQL connector. The primary purpose is to consolidate HTTP client usage, eliminating various singleton HTTP clients and direct requests
library usage in favor of a single UnifiedHttpClient
based on urllib3.
Key changes:
- Introduced
UnifiedHttpClient
class that provides a centralized HTTP client with retry policies, connection pooling, SSL support, and proxy support - Removed singleton HTTP clients (
DatabricksHttpClient
,TelemetryHttpClient
) and replaced directrequests
usage - Updated all components (auth, telemetry, staging operations, cloud fetch, feature flags) to use the unified client
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/databricks/sql/common/unified_http_client.py |
New unified HTTP client implementation using urllib3 |
src/databricks/sql/client.py |
Updated to create and use unified HTTP client, replaced direct requests usage |
src/databricks/sql/session.py |
Modified to accept and use unified HTTP client |
src/databricks/sql/common/http.py |
Removed singleton HTTP client classes |
src/databricks/sql/auth/*.py |
Updated auth providers to use unified HTTP client |
src/databricks/sql/telemetry/telemetry_client.py |
Refactored to use unified client with singleton pattern |
tests/unit/*.py |
Updated tests to mock unified HTTP client |
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for making the changes
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
What type of PR is this?
Description
This PR refactors the codebase to use a unified HTTP client across all HTTP operations in the Databricks SQL connector. The primary purpose is to consolidate HTTP client usage, eliminating various singleton HTTP clients and direct requests library usage in favor of a single UnifiedHttpClient based on urllib3.
Key changes:
How is this tested?
Related Tickets & Documents