-
Notifications
You must be signed in to change notification settings - Fork 25
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
Support pipeline factory functions as CLI reference #699
Conversation
TIL |
I'm going to review this tomorrow after a nights sleep 😉 |
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.
Cool! Going to open a lot op possibilities
# Attribute is not a valid python name | ||
"examples.example_modules.pipeline:pipe;line", | ||
# Not a Pipeline | ||
"examples.example_modules.pipeline:number", | ||
], | ||
) | ||
def test_pipeline_from_module_error(module_str): | ||
"""Test different error cases for pipeline_from_string.""" | ||
with pytest.raises(PipelineImportError): |
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.
can't you match the error messages here ? with pytest match
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.
Should be possible, but I didn't think it was worth the effort. Prioritizing some other work at the moment :)
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.
ok to merge this then if it blocking
In our RAG use case, we want to start using a pipeline factory function, but this is currently not supported by the CLI.
This PR fixes that.
I also included a change (the first commit) which removes the
finally
blocks. We were using this as follows:But apparently the
finally
block is also entered if any non-SpecificError
exception is raised in thetry
block. This lead to all non-handled errors to become anUnboundLocal
error. This is now no longer the case, and the original error is not caught.