feat(csharp/src/Drivers/Databricks): Integrate ProxyConfigurations with Drivers #2794

toddmeng-db · 2025-05-08T00:03:31Z

Integrates proxy logic with the rest of the Spark-Driver. Also applies to Cloud-fetch and OAuth2 fetch.

Other drivers, like Impala driver, should be compatible with the change. However, it is not tested - so I marked in the README that it's not fully complete

To test, add the following to DATABRICKS_TEST_CONFIG_FILE environment variable:

  "http_options": {
    "proxy": {
      "use_proxy": "true",
      "proxy_host": "...",
      "proxy_port":..,
      "proxy_auth": "...", // if using basic auth
      "proxy_uid": "...",
      "proxy_pwd": "..."
    },
    "tls": { // you may need to enable depending on your proxy, or you need to trust the cert
      "enabled": ,
      "allow_self_signed": 
    }
  }

And inspect proxy traffic. Verified that it works in driver testing and Powerbi testing

move to spark layer unit tests fixes message handler factory refactor to hiveserver2 tls refactor delete old files revert http client factory fix proxy option names feedback fixes - internal access implement proxy remove proxy integration test

toddmeng-db · 2025-05-14T21:15:43Z

csharp/src/Drivers/Apache/Hive2/HiveServer2ProxyConfigurator.cs

                if (proxyPort == null)
                    throw new ArgumentNullException(nameof(proxyPort));
            }



small improvement; forgot to check for useProxy here

toddmeng-db · 2025-05-14T22:52:14Z

csharp/test/Drivers/Apache/Hive2/HiveServer2TestEnvironment.cs

                    {
                        parameters.Add(HttpTlsOptions.TrustedCertificatePath, tlsOptions.TrustedCertificatePath!);
                    }
                }


In a seperate PR, Will take a look into creating some BaseTestEnvironment to reduce the code re-use here

csharp/src/Drivers/Apache/Hive2/HiveServer2HttpConnection.cs

toddmeng-db · 2025-05-16T22:00:49Z

csharp/src/Drivers/Apache/Hive2/README.md

+| `adbc.proxy_options.proxy_auth` | Whether to enable proxy authentication. Only feature-complete in Spark driver. One of `True`, `False` | `False` |
+| `adbc.proxy_options.proxy_uid` | Username for proxy authentication. Only feature-complete in Spark driver. Required when proxy_auth is True | |
+| `adbc.proxy_options.proxy_pwd` | Password for proxy authentication. Only feature-complete in Spark driver. Required when proxy_auth is True | |



I think this will work out of the box for impala - but since I have not tested it, hence "Only feature-complete in Spark driver"

eric-wang-1990

LGTM

CurtHagenlocher

Thanks; looks good overall. What do you want to do about the cloud fetch timeout?

CurtHagenlocher · 2025-05-19T16:11:38Z

csharp/src/Drivers/Databricks/CloudFetch/CloudFetchDownloadManager.cs

-            {
-                Timeout = TimeSpan.FromMinutes(timeoutMinutes)
-            };
+            _httpClient = httpClient;


After this change, it appears that the CloudFetchTimeoutMinutes parameter isn't used any more. Should it be removed entirely? I don't have enough experience with HttpClient to know how (or whether) a single client can be used with different timeouts in different contexts.

Ah that's a good point. I'm assuming we want CloudFetch timeout to stay uniquely configurable because it can take a long time to consume large results. Let me update the PR

When we change the timeout on the shared client, it will also affect the other users of the client -- including, I think, the polling to keep the session alive and any subsequent commands issued for this connection. Does that seem okay? Should it be reset once the cloud fetch is done?

I think to support different timeouts on the same HttpClient they would need to use the cancellation token on GetAsync, but if both HttpClient.Timeout and a CancellationToken are used then whichever one triggers first will cancel the request.

I believe the CloudFetch Http Client and the other http clients are still different HttpClients - so this commit cd636cd should only affect the CloudFetch http timeout?

Ah, okay, I missed that.

CurtHagenlocher · 2025-05-19T16:12:37Z

csharp/test/Drivers/Databricks/Auth/OAuthClientCredentialsProviderTests.cs

 using Xunit;
 using Xunit.Abstractions;
 using Apache.Arrow.Adbc.Tests.Drivers.Databricks;
+using System.Net.Http;


nit: sort usings

CurtHagenlocher

Thanks!

toddmeng-db force-pushed the toddmeng-db/proxy-integrate branch from dae4d22 to 2647db0 Compare May 8, 2025 16:52

toddmeng-db closed this May 14, 2025

toddmeng-db reopened this May 14, 2025

Proxy configuration

644c31a

move to spark layer unit tests fixes message handler factory refactor to hiveserver2 tls refactor delete old files revert http client factory fix proxy option names feedback fixes - internal access implement proxy remove proxy integration test

toddmeng-db force-pushed the toddmeng-db/proxy-integrate branch from 2647db0 to 0509832 Compare May 14, 2025 21:14

toddmeng-db commented May 14, 2025

View reviewed changes

toddmeng-db force-pushed the toddmeng-db/proxy-integrate branch 5 times, most recently from 2b7af16 to 5a2b044 Compare May 14, 2025 22:47

toddmeng-db commented May 14, 2025

View reviewed changes

toddmeng-db force-pushed the toddmeng-db/proxy-integrate branch from 5a2b044 to def410c Compare May 14, 2025 22:54

toddmeng-db changed the title ~~Toddmeng db/proxy integrate~~ feat(csharp/src/Drivers/Databricks): Integrate ProxyConfigurations with Drivers May 15, 2025

toddmeng-db commented May 15, 2025

View reviewed changes

csharp/src/Drivers/Apache/Hive2/HiveServer2HttpConnection.cs Show resolved Hide resolved

toddmeng-db force-pushed the toddmeng-db/proxy-integrate branch from def410c to 5f12763 Compare May 15, 2025 01:54

implementation

59c6175

toddmeng-db force-pushed the toddmeng-db/proxy-integrate branch from 5f12763 to 59c6175 Compare May 15, 2025 02:05

toddmeng-db marked this pull request as ready for review May 15, 2025 02:13

toddmeng-db requested a review from CurtHagenlocher as a code owner May 15, 2025 02:13

github-actions bot added this to the ADBC Libraries 19 milestone May 15, 2025

toddmeng-db marked this pull request as draft May 15, 2025 02:15

Readme update

5180ceb

toddmeng-db marked this pull request as ready for review May 15, 2025 02:20

toddmeng-db added 2 commits May 16, 2025 10:38

document that only spark driver fully supports

be0005e

update proxy comment

65a926f

toddmeng-db commented May 16, 2025

View reviewed changes

eric-wang-1990 approved these changes May 16, 2025

View reviewed changes

CurtHagenlocher requested changes May 19, 2025

View reviewed changes

fixes

cd636cd

toddmeng-db force-pushed the toddmeng-db/proxy-integrate branch from 7f7a345 to cd636cd Compare May 19, 2025 17:14

toddmeng-db requested a review from CurtHagenlocher May 19, 2025 17:25

CurtHagenlocher approved these changes May 19, 2025

View reviewed changes

CurtHagenlocher merged commit 148bc17 into apache:main May 19, 2025
7 checks passed

serramatutu mentioned this pull request Aug 12, 2025

Sync upstream dbt-labs/arrow-adbc#52

Closed

feat(csharp/src/Drivers/Databricks): Integrate ProxyConfigurations with Drivers #2794

feat(csharp/src/Drivers/Databricks): Integrate ProxyConfigurations with Drivers #2794

Uh oh!

Conversation

toddmeng-db commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toddmeng-db May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

toddmeng-db May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wang-1990 left a comment

Choose a reason for hiding this comment

Uh oh!

CurtHagenlocher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CurtHagenlocher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

toddmeng-db commented May 8, 2025 •

edited

Loading

toddmeng-db May 14, 2025 •

edited

Loading

toddmeng-db May 16, 2025 •

edited

Loading