Lee-W commented on code in PR #72156:
URL: https://github.com/apache/airflow/pull/72156#discussion_r4045836630
##########
providers/common/ai/src/airflow/providers/common/ai/hooks/pydantic_ai.py:
##########
@@ -178,24 +303,101 @@ def _provider_factory(pname: str) -> Any:
)
return infer_provider(pname)
- self._model = infer_model(model_name,
provider_factory=_provider_factory)
- return self._model
+ return infer_model(model_name, provider_factory=_provider_factory)
- self._model = infer_model(model_name)
- return self._model
+ return infer_model(model_name)
- def _get_conn_if_model_configured(self) -> Model | None:
- """Return the hook model only when the hook or connection explicitly
configures one."""
- if self.model_id:
- return self.get_conn()
+ def _get_fallback_conn_ids(self) -> list[str]:
+ """Return the configured fallback connection IDs, hook argument
winning over the extra."""
+ if self.fallback_conn_ids is not None:
+ raw: Any = self.fallback_conn_ids
+ else:
+ _, extra = self._get_conn_and_extra()
+ raw = extra.get(FALLBACK_CONN_IDS_EXTRA_KEY)
+ if raw is None:
+ raw = []
+
+ if not isinstance(raw, (list, tuple)) or not all(isinstance(item, str)
and item for item in raw):
Review Comment:
Blank and whitespace-only entries are dropped and the survivors stripped;
non-string entries still raise. The `__init__` docstring says so as well — the
behaviour being invisible was the actual problem, and it would still have been
invisible in the rendered docs.
##########
providers/common/ai/src/airflow/providers/common/ai/hooks/pydantic_ai.py:
##########
@@ -178,24 +303,101 @@ def _provider_factory(pname: str) -> Any:
)
return infer_provider(pname)
- self._model = infer_model(model_name,
provider_factory=_provider_factory)
- return self._model
+ return infer_model(model_name, provider_factory=_provider_factory)
- self._model = infer_model(model_name)
- return self._model
+ return infer_model(model_name)
- def _get_conn_if_model_configured(self) -> Model | None:
- """Return the hook model only when the hook or connection explicitly
configures one."""
- if self.model_id:
- return self.get_conn()
+ def _get_fallback_conn_ids(self) -> list[str]:
+ """Return the configured fallback connection IDs, hook argument
winning over the extra."""
+ if self.fallback_conn_ids is not None:
+ raw: Any = self.fallback_conn_ids
+ else:
+ _, extra = self._get_conn_and_extra()
+ raw = extra.get(FALLBACK_CONN_IDS_EXTRA_KEY)
+ if raw is None:
+ raw = []
+
+ if not isinstance(raw, (list, tuple)) or not all(isinstance(item, str)
and item for item in raw):
+ raise ValueError(
+ f"{FALLBACK_CONN_IDS_EXTRA_KEY} for connection
'{self.llm_conn_id}' must be a list "
+ f"of non-empty connection IDs, got {raw!r}."
+ )
+ return list(raw)
- conn = self.get_connection(self.llm_conn_id)
- self._conn = conn
- self._conn_extra_dejson = conn.extra_dejson
+ def _resolve_fallback_models(self) -> list[Model]:
+ """
+ Resolve one ``Model`` per fallback connection, in the configured order.
Review Comment:
Counted six copies rather than four once the other changes in this round
landed. The rule lives in `_has_recognized_provider_prefix` and the class
docstring now, with the rest pointing there. Four mentions of the Bedrock id
remain and each carries a different argument: the detection rule itself, why
the slug pattern excludes a dot, the public `model_id` docs, and cross-platform
gating.
--
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]
