-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
test: enhance e2e tests to also draw and serialize/deserialize the test pipelines #5910
Conversation
Pull Request Test Coverage Report for Build 6407362412
💛 - Coveralls |
@@ -121,7 +121,7 @@ def from_dict(cls, data: Dict[str, Any]) -> "GPTGenerator": | |||
""" | |||
init_params = data.get("init_parameters", {}) | |||
streaming_callback = None | |||
if "streaming_callback" in init_params: | |||
if "streaming_callback" in init_params and init_params["streaming_callback"]: |
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.
This bug was found when I introduced serialization for the RAG pipelines. Maybe worth adding a unit test too?
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.
Yes please, in a separate PR.
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.
LGTM! 👍 I mentioned an alternative, smaller language model in a comment. If you want, you could try it out. Keeping it as is is fine. 🙂
@@ -121,7 +121,7 @@ def from_dict(cls, data: Dict[str, Any]) -> "GPTGenerator": | |||
""" | |||
init_params = data.get("init_parameters", {}) | |||
streaming_callback = None | |||
if "streaming_callback" in init_params: | |||
if "streaming_callback" in init_params and init_params["streaming_callback"]: |
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.
Yes please, in a separate PR.
Related Issues
pipeline.draw()
broken for components with objects in their state canals#108Proposed Changes:
e2e/preview/pipelines
.How did you test it?
Notes for the reviewer
Bugs that are discovered should be fixed in parallel PRs before merging this one
Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
.