kaxil commented on PR #62816:
URL: https://github.com/apache/airflow/pull/62816#issuecomment-4000519830
Thanks for the follow-up! I think there's a misunderstanding — I'm not
suggesting env-vars-only. The hook already reads credentials from the Airflow
Connection (`conn.password` → `api_key`, `conn.host` → `base_url`). The env-var
path only kicks in when no credentials are set on the connection (for
Bedrock/Vertex users who rely on IAM auth).
What I'm suggesting is: instead of introspecting provider constructors with
`inspect.signature`, just add explicit branches for the providers that need
special handling. Right now that's only Azure and LiteLLM. For example,
something along these lines in `_provider_factory`:
```python
provider_cls = infer_provider_class(provider_name)
if provider_name == "azure":
return provider_cls(azure_endpoint=base_url, api_key=api_key)
if provider_name == "litellm":
return provider_cls(api_base=base_url, api_key=api_key)
# Default path — works for OpenAI, Anthropic, Groq, Mistral, etc.
kwargs = {}
if api_key:
kwargs["api_key"] = api_key
if base_url:
kwargs["base_url"] = base_url
return provider_cls(**kwargs)
```
This is explicit, easy to read, and doesn't break if pydantic-ai renames a
constructor parameter. The `inspect.signature` approach is clever but fragile —
if a provider adds or renames a param, the introspection silently changes
behavior. Explicit branches make the mapping obvious and reviewable.
For connection extras: I'm fine with passing extra fields through to the
provider constructor for the specific branches that need them (e.g.
`api_version` for Azure). But that should be per-provider, not a generic "dump
all extras into kwargs" pattern.
--
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]
