kaxil commented on code in PR #74451:
URL: https://github.com/apache/airflow/pull/74451#discussion_r4352755181
##########
providers/common/ai/src/airflow/providers/common/ai/hooks/llamaindex.py:
##########
@@ -115,9 +116,9 @@ def __init__(
self,
llm_conn_id: str | None = None,
embed_conn_id: str | None = None,
- embed_model: str | None = None,
- llm_model: str | None = None,
*,
Review Comment:
1.0.0 shipped on 2026-10-19 with `llm_model` and `embed_model` positional
here. `LlamaIndexHook("c", None, "text-embedding-3-small")` worked on 1.0.0 and
raises `TypeError` with this change. Both hooks are experimental in
`stability.rst`, and that page asks for breaking changes to experimental
features to be announced in the changelog. Could you add a note under the
Changelog header in `docs/changelog.rst`? I'd put the fail-closed `ValueError`
from `PydanticAIHook` in it too, and the retry module's switch to
`AirflowOptionalProviderFeatureException`: the 1.0.0 example used `except
ImportError:`, so Dags that copied it need a wider guard on 2.11 to 3.2. The PR
body also still says "heading to 1.0", and the Breaking bullet reads
"`LlamaIndexHookd_model`".
##########
providers/common/ai/src/airflow/providers/common/ai/hooks/pydantic_ai.py:
##########
@@ -451,13 +451,12 @@ def _resolve_own_model(
def _provider_factory(pname: str) -> Any:
try:
return infer_provider_class(pname)(**_kwargs)
- except TypeError:
- self.log.warning(
- "Provider '%s' rejected kwargs %s; falling back to
env-var auth",
- pname,
- list(_kwargs),
- )
- return infer_provider(pname)
+ except TypeError as e:
+ raise ValueError(
+ f"Could not build provider '{pname}' from the fields
on connection "
Review Comment:
The rationale comment in `tests/unit/common/ai/test_provider_metadata.py`
(lines 136-143) still says `_provider_factory` catches this `TypeError` and
retries via `infer_provider(pname)`, and that the Codex CLI login works "via
that fallback". With this change an `openai-codex:` model on a connection with
a password or host raises here, and the ambient login only applies when neither
is set. The conclusion still holds; just the mechanism it describes is gone.
--
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]
