-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat: validation function for run()
and run_async()
parameters signature for (custom) components
#9322
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
feat: validation function for run()
and run_async()
parameters signature for (custom) components
#9322
Conversation
run()
and run_async()
parameters signature for (custom) components
Pull Request Test Coverage Report for Build 14732755474Details
💛 - Coveralls |
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.
I left two minor comments, but looks good to me!
|
||
|
||
def test_different_param_names(): | ||
with pytest.raises(ComponentError) as exc_info: |
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.
nit: you could use something like with pytest.raises(ComponentError, match="name mismatch")
in all tests if you want...
https://docs.pytest.org/en/stable/how-to/assert.html#matching-exception-messages
haystack/core/component/component.py
Outdated
@@ -326,6 +329,48 @@ def _component_run_has_kwargs(component_cls: Type) -> bool: | |||
) | |||
|
|||
|
|||
def _compare_method_signatures(run_sig: inspect.Signature, async_run_sig: inspect.Signature) -> str: |
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.
def _compare_method_signatures(run_sig: inspect.Signature, async_run_sig: inspect.Signature) -> str: | |
def _compare_run_methods_signatures(run_sig: inspect.Signature, async_run_sig: inspect.Signature) -> str: |
nit: maybe this could be a clearer name
Related Issues
Proposed Changes:
Component signature validation mismatches between
run
andrun_async
method signatures, this is bound to happen with custom components as all haystack core and integration components by design don't have this issue._compare_method_signatures
that provides detailed information about signature mismatches about:How did you test it?
test_component_signature_validation.py
with comprehensive test cases cover all possible signature mismatch scenarios described aboveChecklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
and added!
in case the PR includes breaking changes.