Vamsi-klu commented on code in PR #69747:
URL: https://github.com/apache/airflow/pull/69747#discussion_r3567697940
##########
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:
yeah good point. it's on purpose so the sqlalchemy url stays fully offline
and doesn't hit the api.
threw a note in the docstring for the resolve function to explain. when both
the endpoint name and http path in the extra are set the url can point at a
different warehouse than what get conn actually uses. tests already cover this
case.
appreciate the review and the live testing you did.
--
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]
