Skip to content

Commit f326287

Browse files
committed
[PECOBLR-1968] Implement fix
1 parent 9fe7356 commit f326287

File tree

2 files changed

+135
-0
lines changed

2 files changed

+135
-0
lines changed

src/databricks/sql/client.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,78 @@
9393
# Transaction isolation level constants (extension to PEP 249)
9494
TRANSACTION_ISOLATION_LEVEL_REPEATABLE_READ = "REPEATABLE_READ"
9595

96+
# All supported **kwargs for Connection.__init__. Used to warn on unknown params.
97+
KNOWN_KWARGS = frozenset(
98+
[
99+
# Authentication
100+
"access_token",
101+
"auth_type",
102+
"username",
103+
"password",
104+
"credentials_provider",
105+
"_use_cert_as_auth",
106+
"oauth_client_id",
107+
"oauth_redirect_port",
108+
"oauth_scopes",
109+
"experimental_oauth_persistence",
110+
"identity_federation_client_id",
111+
"azure_client_id",
112+
"azure_client_secret",
113+
"azure_tenant_id",
114+
"azure_workspace_resource_id",
115+
# TLS / SSL
116+
"_enable_ssl",
117+
"_tls_no_verify",
118+
"_tls_verify_hostname",
119+
"_tls_trusted_ca_file",
120+
"_tls_client_cert_file",
121+
"_tls_client_cert_key_file",
122+
"_tls_client_cert_key_password",
123+
# Connection
124+
"_port",
125+
"_connection_uri",
126+
"_skip_routing_headers",
127+
# Retry policy
128+
"_retry_delay_min",
129+
"_retry_delay_max",
130+
"_retry_delay_default",
131+
"_retry_stop_after_attempts_count",
132+
"_retry_stop_after_attempts_duration",
133+
"_retry_dangerous_codes",
134+
"_retry_max_redirects",
135+
"_enable_v3_retries",
136+
# Socket / network
137+
"_socket_timeout",
138+
"_proxy_auth_method",
139+
"_pool_connections",
140+
"_pool_maxsize",
141+
"pool_maxsize",
142+
# User agent & telemetry
143+
"user_agent_entry",
144+
"_user_agent_entry",
145+
"enable_telemetry",
146+
"force_enable_telemetry",
147+
"telemetry_batch_size",
148+
"_telemetry_circuit_breaker_enabled",
149+
# Query / result
150+
"use_inline_params",
151+
"enable_query_result_lz4_compression",
152+
"use_cloud_fetch",
153+
"max_download_threads",
154+
"use_hybrid_disposition",
155+
"staging_allowed_local_path",
156+
# Data type / format
157+
"_use_arrow_native_decimals",
158+
"_use_arrow_native_timestamps",
159+
"_disable_pandas",
160+
# Behavior
161+
"enable_metric_view_metadata",
162+
"fetch_autocommit_from_server",
163+
# Backend selection
164+
"use_sea",
165+
]
166+
)
167+
96168

97169
class Connection:
98170
def __init__(
@@ -269,6 +341,14 @@ def read(self) -> Optional[OAuthToken]:
269341
http_path,
270342
)
271343

344+
unknown_params = set(kwargs.keys()) - KNOWN_KWARGS
345+
if unknown_params:
346+
logger.warning(
347+
"Unsupported connection parameter(s) will be ignored: %s. "
348+
"Check the Connection documentation for supported parameters.",
349+
", ".join(sorted(unknown_params)),
350+
)
351+
272352
if access_token:
273353
access_token_kv = {"access_token": access_token}
274354
kwargs = {**kwargs, **access_token_kv}

tests/unit/test_client.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,61 @@ def mock_close_normal():
633633
cursors_closed, [1, 2], "Both cursors should have close called"
634634
)
635635

636+
@patch("%s.session.ThriftDatabricksClient" % PACKAGE_NAME)
637+
@patch("databricks.sql.client.logger")
638+
def test_unknown_connection_param_issues_warning(
639+
self, mock_logger, mock_client_class
640+
):
641+
"""Unknown kwargs passed to Connection should trigger a warning."""
642+
databricks.sql.connect(
643+
**self.DUMMY_CONNECTION_ARGS, totally_unknown_param="value"
644+
)
645+
mock_logger.warning.assert_called_once()
646+
warning_msg = mock_logger.warning.call_args[0][0]
647+
self.assertIn("Unsupported connection parameter", warning_msg)
648+
649+
@patch("%s.session.ThriftDatabricksClient" % PACKAGE_NAME)
650+
@patch("databricks.sql.client.logger")
651+
def test_unknown_connection_param_warning_names_the_param(
652+
self, mock_logger, mock_client_class
653+
):
654+
"""Warning message should include the name of the unknown parameter."""
655+
databricks.sql.connect(
656+
**self.DUMMY_CONNECTION_ARGS, totally_unknown_param="value"
657+
)
658+
# The unknown param name should appear in the warning args
659+
call_args = mock_logger.warning.call_args
660+
formatted_msg = call_args[0][0] % call_args[0][1:]
661+
self.assertIn("totally_unknown_param", formatted_msg)
662+
663+
@patch("%s.session.ThriftDatabricksClient" % PACKAGE_NAME)
664+
@patch("databricks.sql.client.logger")
665+
def test_known_connection_params_do_not_issue_warning(
666+
self, mock_logger, mock_client_class
667+
):
668+
"""Known kwargs passed to Connection should not trigger an unknown-param warning."""
669+
databricks.sql.connect(**self.DUMMY_CONNECTION_ARGS, use_cloud_fetch=False)
670+
for call in mock_logger.warning.call_args_list:
671+
msg = call[0][0]
672+
self.assertNotIn("Unsupported connection parameter", msg)
673+
674+
@patch("%s.session.ThriftDatabricksClient" % PACKAGE_NAME)
675+
@patch("databricks.sql.client.logger")
676+
def test_multiple_unknown_params_all_appear_in_warning(
677+
self, mock_logger, mock_client_class
678+
):
679+
"""All unknown param names should appear in the warning message."""
680+
databricks.sql.connect(
681+
**self.DUMMY_CONNECTION_ARGS,
682+
bad_param_one="a",
683+
bad_param_two="b",
684+
)
685+
mock_logger.warning.assert_called_once()
686+
call_args = mock_logger.warning.call_args
687+
formatted_msg = call_args[0][0] % call_args[0][1:]
688+
self.assertIn("bad_param_one", formatted_msg)
689+
self.assertIn("bad_param_two", formatted_msg)
690+
636691

637692
class TransactionTestSuite(unittest.TestCase):
638693
"""

0 commit comments

Comments
 (0)