jeff3071 commented on code in PR #71437:
URL: https://github.com/apache/airflow/pull/71437#discussion_r3979280503
##########
providers/common/ai/src/airflow/providers/common/ai/hooks/pydantic_ai.py:
##########
@@ -127,6 +138,35 @@ def _get_provider_kwargs(
kwargs["base_url"] = base_url
return kwargs
+ def _get_cached_connection(self, conn_id: str) -> Connection:
+ if conn_id not in self._connections:
+ self._connections[conn_id] = self.get_connection(conn_id)
+ return self._connections[conn_id]
+
+ def _warn_if_vertexai_field_ignored(self, extra: dict[str, Any]) -> None:
+ if extra.get("vertexai") is not None:
+ self.log.warning(
+ "The 'vertexai' connection field is ignored; Vertex AI vs.
Generative Language "
+ "API mode is now selected via the model prefix
('google-cloud:' vs. 'google:')."
+ )
+
+ def _get_provider_kwargs_for_model(self, conn: Connection, model_name:
str) -> dict[str, Any]:
+ provider_name, _ = parse_model_id(model_name)
+ provider_kwargs_mapper = _PROVIDER_KWARGS_MAPPER_BY_MODEL_PREFIX.get(
+ provider_name, PydanticAIHook._get_provider_kwargs
+ )
+ extra = conn.extra_dejson
+ self._warn_if_vertexai_field_ignored(extra)
Review Comment:
Here is my concern.
For example, consider a generic connection configured with:
```
{
"conn_type": "pydanticai",
"password": "sk-openai-...",
"host": "https://api.openai.com/v1";,
"extra": {
"model": "openai:gpt-5.6-sol",
"embed_model": "google:text-embedding-004"
}
}
```
Here, password and host belong to the OpenAI model. If the Google mapper
falls back to those common connection fields, it would also pass the OpenAI API
key and endpoint to GooglefinderProvider. That is why I chose to use only
provider-specific values from extra for Bedrock and Google rather than
implicitly sharing the generic credentials.
I agree that silently dropping documented fields is surprising, though,
particularly when the generic connection contains only a Bedrock or Google
model.
Would you prefer that we preserve the generic connection behavior by falling
back to `conn.password` and `conn.host` when the corresponding
provider-specific values are absent from extra, accepting that users who mix
providers should configure a separate `embed_conn_id`?
--
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]
