Skip to content

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

Merged
merged 16 commits into from
Aug 13, 2025

Conversation

vikrantpuppala
Copy link
Contributor

@vikrantpuppala vikrantpuppala commented Aug 8, 2025

What type of PR is this?

  • Refactor
  • Feature
  • Bug Fix
  • Other

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:

  • 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 direct requests usage
  • Updated all components (auth, telemetry, staging operations, cloud fetch, feature flags) to use the unified client

How is this tested?

  • Unit tests
  • E2E Tests
  • Manually tested auth flows
  • N/A

Related Tickets & Documents

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>
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>
Copy link

@Copilot Copilot AI left a 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 direct requests 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>
Copy link
Contributor

@jprakash-db jprakash-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. 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>
@vikrantpuppala vikrantpuppala merged commit d3df719 into main Aug 13, 2025
24 of 25 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