kaxil commented on code in PR #74451:
URL: https://github.com/apache/airflow/pull/74451#discussion_r4289938421
##########
providers/common/ai/src/airflow/providers/common/ai/hooks/mcp.py:
##########
@@ -236,13 +237,41 @@ def get_conn(self) -> Any:
raise ValueError(
f"Connection {self.mcp_conn_id!r} requires 'command' in
extra for stdio transport."
)
- args = extra.get("args", [])
- if isinstance(args, str):
- args = [args]
+ args = extra.get("args")
+ if args is None:
+ args = []
+ elif isinstance(args, str):
+ if args.strip().startswith("["):
+ try:
+ args = json.loads(args)
+ except ValueError as e:
+ raise ValueError(
+ f"'args' in extra for connection
{self.mcp_conn_id!r} looks like a JSON array "
+ f"but is not valid JSON: {args!r}."
+ ) from e
+ else:
+ args = [args]
+ if not isinstance(args, list) or not all(isinstance(arg, str) for
arg in args):
+ raise ValueError(
+ f"'args' in extra for connection {self.mcp_conn_id!r} must
be a string, a JSON "
+ f"array of strings or a list of strings, got {args!r}."
Review Comment:
This message and the invalid-JSON one above print the whole `args` value,
and nothing masks it: the secrets masker only registers extra values under
sensitive key names, and `args` isn't one. So a stdio connection like
`mcp-remote https://... --header "Authorization: Bearer <tok>"` with one item
typed as a number puts the token in the task log. `_validate_env_dict` already
reports only the type and key; could these report the type and index of the bad
item instead, and drop `{args!r}` from the JSON error?
##########
providers/common/ai/src/airflow/providers/common/ai/hooks/pydantic_ai.py:
##########
@@ -875,8 +873,11 @@ class PydanticAIVertexHook(PydanticAIHook):
above), so its presence alone cannot tell the two platforms apart; guessing
would risk silently authenticating against the wrong endpoint. To use the
Generative Language API, set an explicit ``google:``-prefixed model id (on
- the hook or the connection's ``model`` extra) -- that spelling already
- works today.
+ the hook or the connection's ``model`` extra). ``GoogleProvider`` accepts
only
Review Comment:
This now contradicts item 1 of the credential order at the top of the
docstring, which says `service_account_info` is passed as `credentials` to
`GoogleProvider`. It goes to `GoogleCloudProvider`, which is the class that
takes `credentials`, `project` and `location`. The `# Direct GoogleProvider
scalar kwargs.` comment above the `project`/`location` loop in
`_get_provider_kwargs` has the same mix-up.
--
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]
