moomindani commented on code in PR #69747:
URL: https://github.com/apache/airflow/pull/69747#discussion_r3567374802
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_sql.py:
##########
@@ -184,15 +184,26 @@ def _get_sql_endpoint_by_name(self, endpoint_name) ->
dict[str, Any]:
else:
return endpoint
+ def _resolve_http_path(self, allow_endpoint_lookup: bool = True) -> str |
None:
+ """
+ Resolve http_path from explicit arg, endpoint name, or connection
extra.
+
+ :param allow_endpoint_lookup: If True, may call API to resolve
sql_endpoint_name.
+ Set False for offline-safe paths like sqlalchemy_url.
+ :return: resolved http_path or None if not found.
+ """
+ if self._http_path:
+ return self._http_path
+ if allow_endpoint_lookup and self._sql_endpoint_name:
+ endpoint = self._get_sql_endpoint_by_name(self._sql_endpoint_name)
+ return endpoint["odbc_params"]["path"]
+ return self.databricks_conn.extra_dejson.get("http_path")
Review Comment:
Non-blocking: when both `sql_endpoint_name` and an extra `http_path` are
set, the offline path (`allow_endpoint_lookup=False`, used by `sqlalchemy_url`)
falls through to the extra value, while `get_conn()` resolves the endpoint name
— so the URL the property reports can point at a different warehouse than the
one the hook actually connects to. Probably acceptable as a best-effort offline
value, but a sentence in the docstring would make the asymmetry clearly
intentional.
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_sql.py:
##########
@@ -184,15 +184,26 @@ def _get_sql_endpoint_by_name(self, endpoint_name) ->
dict[str, Any]:
else:
return endpoint
+ def _resolve_http_path(self, allow_endpoint_lookup: bool = True) -> str |
None:
+ """
+ Resolve http_path from explicit arg, endpoint name, or connection
extra.
+
+ :param allow_endpoint_lookup: If True, may call API to resolve
sql_endpoint_name.
+ Set False for offline-safe paths like sqlalchemy_url.
+ :return: resolved http_path or None if not found.
+ """
+ if self._http_path:
+ return self._http_path
+ if allow_endpoint_lookup and self._sql_endpoint_name:
+ endpoint = self._get_sql_endpoint_by_name(self._sql_endpoint_name)
+ return endpoint["odbc_params"]["path"]
+ return self.databricks_conn.extra_dejson.get("http_path")
+
def get_conn(self) -> AirflowConnection:
"""Return a Databricks SQL connection object."""
if not self._http_path:
Review Comment:
Nit: this guard duplicates the helper's first check (`if self._http_path:
return self._http_path`) — calling the helper unconditionally and assigning the
result behaves identically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
