wenjin272 commented on code in PR #1051:
URL: https://github.com/apache/flink-agents/pull/1051#discussion_r3861526318
##########
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:
Could we avoid adding `name` as an accepted chat-model field? The in-tree
`name=` call sites are all tests, while the resource name is owned by
`ResourceProvider` and is not passed into the resource during construction.
This therefore preserves a previously ignored, behaviorless argument solely to
keep those tests passing, which works against the goal of rejecting unsupported
inputs.
I think these tests should drop `name=` instead of adding a no-op exception
to strict validation.
--
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]
