-
Notifications
You must be signed in to change notification settings - Fork 2.7k
feat: Add ability to set messages as required in Agent through required_variables
#11062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -261,7 +261,8 @@ def inner(method: Callable[..., Any], sockets: Sockets) -> inspect.Signature: | |
| existing_socket = sockets.get(param_name) | ||
| if existing_socket is not None and existing_socket != new_socket: | ||
| raise ComponentError( | ||
| "set_input_types()/set_input_type() cannot override the parameters of the 'run' method" | ||
| "set_input_types()/set_input_type() cannot override the parameters of the 'run' method.\n" | ||
| f"Conflict found for parameter '{param_name}': {existing_socket} vs {new_socket}" | ||
|
Comment on lines
263
to
+265
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @julian-risch this is what throws an error if the set_input_type is done in the init method. |
||
| ) | ||
|
|
||
| sockets[param_name] = new_socket | ||
|
|
@@ -322,6 +323,11 @@ def __call__(cls, *args: Any, **kwargs: Any) -> Any: | |
| ComponentMeta._parse_and_set_input_sockets(cls, instance) | ||
| ComponentMeta._parse_and_set_output_sockets(instance) | ||
|
|
||
| # Call __post_init__ if defined, allowing components to adjust sockets after | ||
| # the run method signature has been parsed (e.g. to make an existing socket required). | ||
| if callable(getattr(instance, "__post_init__", None)): | ||
| instance.__post_init__() | ||
|
Comment on lines
+326
to
+329
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @julian-risch this is what I'm most concerned about since one could argue it's a pretty big feature to add support for |
||
|
|
||
| # Since a Component can't be used in multiple Pipelines at the same time | ||
| # we need to know if it's already owned by a Pipeline when adding it to one. | ||
| # We use this flag to check that. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@julian-risch a
__post_init__was needed to do this because we are unable to runset_input_typeon a param in the run method that is positional and already has a type.