Ashfaqbs commented on code in PR #1051:
URL: https://github.com/apache/flink-agents/pull/1051#discussion_r3862270380
##########
python/flink_agents/api/chat_models/chat_model.py:
##########
@@ -115,6 +115,17 @@ class BaseChatModelConnection(Resource, ABC):
One connection can be shared in multiple chat model setup.
"""
+ # Reject unrecognized constructor arguments instead of silently ignoring
them
+ # (pydantic's default extra="ignore"), so a misspelled or unsupported
config
+ # key fails loudly at construction time instead of appearing to apply and
+ # then having no effect. `name` is declared below purely so the resource
+ # name callers and the resource provider commonly pass through survives
+ # this check; it is not otherwise used by this class.
+ model_config = ConfigDict(arbitrary_types_allowed=True, extra="forbid")
Review Comment:
Good catch, thanks. Fixed by overriding `model_config` back to
`extra="ignore"` on `JavaChatModelConnection`/`JavaChatModelSetup` (the
Java-backed subclasses), so strict validation stays scoped to Python-native
chat models and Java descriptors keep passing through `java_clazz`,
`extract_reasoning`, etc. as before. Pushed in `c971dee7`.
##########
python/flink_agents/api/chat_models/chat_model.py:
##########
@@ -115,6 +115,17 @@ class BaseChatModelConnection(Resource, ABC):
One connection can be shared in multiple chat model setup.
"""
+ # Reject unrecognized constructor arguments instead of silently ignoring
them
+ # (pydantic's default extra="ignore"), so a misspelled or unsupported
config
+ # key fails loudly at construction time instead of appearing to apply and
+ # then having no effect. `name` is declared below purely so the resource
+ # name callers and the resource provider commonly pass through survives
+ # this check; it is not otherwise used by this class.
+ model_config = ConfigDict(arbitrary_types_allowed=True, extra="forbid")
+ name: str | None = Field(
Review Comment:
Agreed, that's cleaner. Removed the `name` field from both base classes and
dropped `name=` from the in-tree test call sites (confirmed `ResourceProvider`
never forwards the resource name into the constructor -- it's kept on the
provider itself). Pushed in `c971dee7`.
--
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]
