You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Subham-KRLX (via GitHub)" <gi...@apache.org> on 2026/06/26 13:00:50 UTC
[PR] Fix DatabricksSqlHook sqlalchemy_url to include http_path from connection extra [airflow]
Subham-KRLX opened a new pull request, #69037:
URL: https://github.com/apache/airflow/pull/69037
This PR fixes #69031 by ensuring that `DatabricksSqlHook.sqlalchemy_url` and `get_uri()` correctly include `http_path` when it is defined in the connection `extra` fields.
We centralized the resolution logic into a helper `_resolve_http_path()`, updated both `get_conn()` and `sqlalchemy_url` to use it, and added corresponding unit tests.
closes: #69031
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes-Claude Sonnet 4.5(For pr description and code research)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix DatabricksSqlHook sqlalchemy_url to include http_path from connection extra [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #69037:
URL: https://github.com/apache/airflow/pull/69037#issuecomment-4956595519
Fixed in https://github.com/apache/airflow/pull/69747
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix DatabricksSqlHook sqlalchemy_url to include http_path from connection extra [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal closed pull request #69037: Fix DatabricksSqlHook sqlalchemy_url to include http_path from connection extra
URL: https://github.com/apache/airflow/pull/69037
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix DatabricksSqlHook sqlalchemy_url to include http_path from connection extra [airflow]
Posted by "moomindani (via GitHub)" <gi...@apache.org>.
moomindani commented on code in PR #69037:
URL: https://github.com/apache/airflow/pull/69037#discussion_r3510566650
##########
providers/databricks/src/airflow/providers/databricks/hooks/databricks_sql.py:
##########
@@ -255,7 +261,7 @@ def sqlalchemy_url(self) -> URL:
)
url_query = {
- "http_path": self._http_path,
+ "http_path": self._resolve_http_path(),
Review Comment:
This line makes `sqlalchemy_url` (a property) perform a REST API call when the hook is configured via `sql_endpoint_name` — see the review body for the failing chain through `dialect_name`/`insert_rows`. Please resolve only from local sources (explicit param, cached value, connection extra) here.
---
Drafted-by: Claude Code (Fable 5); reviewed by @moomindani before posting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix DatabricksSqlHook sqlalchemy_url to include http_path from connection extra [airflow]
Posted by "belegdol (via GitHub)" <gi...@apache.org>.
belegdol commented on PR #69037:
URL: https://github.com/apache/airflow/pull/69037#issuecomment-4862712202
> Suggestion: keep the API lookup out of the property. For example, give the helper a flag — `_resolve_http_path(allow_endpoint_lookup: bool = True)` — where `get_conn()` uses the default, and `sqlalchemy_url` passes `allow_endpoint_lookup=False`, falling back to the previous behavior of omitting `http_path` when only `sql_endpoint_name` is configured and no cached value exists yet (raising there would reject a valid configuration). The connection-extra resolution — the actual #69031 fix — needs no API call and stays as-is.
While it would fix #69031, I believe the error will still be there if one defines the hook with `sql_endpoint` defined instead of the `http_path` - which is supported according to the configuration:
> http_path ([str](https://docs.python.org/3/library/stdtypes.html#str) | None) – Optional string specifying HTTP path of Databricks SQL Endpoint or cluster. If not specified, it should be either specified in the Databricks connection’s extra parameters, or sql_endpoint_name must be specified.
> sql_endpoint_name ([str](https://docs.python.org/3/library/stdtypes.html#str) | None) – Optional name of Databricks SQL Endpoint. If not specified, http_path must be provided as described above.
Would calling `_resolve_http_path` during `init()` be an acceptable solution to this?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org