-
Notifications
You must be signed in to change notification settings - Fork 13
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
fix: raise error on import of AsyncPipeline
#178
Conversation
Pull Request Test Coverage Report for Build 12990000451Details
💛 - 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.
LGTM! 👍 For now, we'll need to pause the experiment on AsyncPipeline. As a next step, we should add a comment to the GitHub discussion about the experimental AsyncPipeline, explain that the experiment is paused, and point to 0.4.0 release. #152
Similarly, we should add a note to the cookbook or remove it entirely: https://github.com/deepset-ai/haystack-cookbook/blob/bd82aac09307379fa082e374d8855769a943b9e7/notebooks/async_pipeline.ipynb
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 - looks good to me - imported the async gave me the expected error, and I was also able to run a pipeline with the new logic.
* raise error for async import * Remove all async pipeline tests
* raise error for async import * Remove all async pipeline tests (cherry picked from commit cbbf088)
Related Issues
Proposed Changes:
RuntimeError
when the user tries to initialize async pipelines in 0.5.0 release.How did you test it?
Ran code manually
Notes for the reviewer
Checklist
fix:
,feat:
,build:
,chore:
,ci:
,docs:
,style:
,refactor:
,perf:
,test:
.