Lee-W commented on code in PR #74451:
URL: https://github.com/apache/airflow/pull/74451#discussion_r4255540892
##########
providers/common/ai/src/airflow/providers/common/ai/hooks/pydantic_ai.py:
##########
@@ -406,13 +406,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"Provider '{pname}' rejected the credentials from
connection "
+ f"'{self.llm_conn_id}' (ignored: {',
'.join(_kwargs)}). Not falling back to "
Review Comment:
Changed it to "fields passed" and included the provider's `TypeError` text.
##########
providers/common/ai/docs/provider_fallback.rst:
##########
@@ -195,15 +195,11 @@ Two checks, neither of which requires waiting for a real
outage:
*Test the connection.* ``test_connection`` on the primary resolves every
connection in the
chain, so a fallback with a missing ``model`` or an unknown connection ID is
reported by name
there rather than discovered mid-incident. Credential fields a provider class
rejects with a
-``TypeError`` are caught by the hook, which retries with the env-var-based
provider
-constructor and logs a warning either way; if the required env var is also
missing, that
-retry raises ``pydantic_ai.exceptions.UserError``, which ``test_connection``
does surface
-since it wraps the whole resolution in a broad exception handler. What it
cannot show is the
-opposite case: the env var *is* set on the worker, the retry quietly succeeds,
and
-``test_connection`` reports success even though the credentials you configured
on the
-connection were silently ignored -- check the logs for that warning rather
than relying on
-``test_connection`` alone. It also does not call the provider, so a
well-formed but revoked
-key still passes -- that is what the drill below is for.
+``TypeError`` fail the resolution with a ``ValueError`` naming the connection,
the provider and
+the ignored fields -- the hook never falls back to environment-variable auth,
which would
Review Comment:
Updated both `self_hosted_models.rst` and `test_cortex_model.py` docstring
to describe the `ValueError` instead of the env-var fallback.
##########
providers/common/ai/src/airflow/providers/common/ai/hooks/mcp.py:
##########
@@ -213,10 +214,10 @@ def get_conn(self) -> Any:
try:
from fastmcp.client.transports import SSETransport,
StdioTransport, StreamableHttpTransport
from pydantic_ai.mcp import MCPToolset
- except ImportError:
- raise ImportError(
+ except ImportError as e:
+ raise AirflowOptionalProviderFeatureException(
Review Comment:
Updated `docs/toolsets/index.rst` to say a missing `mcp` extra raises
`AirflowOptionalProviderFeatureException`, merged with the sentence about the
other toolsets.
--
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]
