-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Reintroduce debug
as a valid global key for Pipeline's params
#2298
Conversation
Please note that haystack/haystack/pipelines/base.py Line 568 in fd46a42
@ZanSara Maybe there is some confusion regarding that? |
AFAIK we allow both options by design. We actually have several ways to enable debug output in a pipeline:
We can argue they're too many ways, but as long as they're documented I think they should all work. In addition, the code looks like all the four options worked well at some point, and this is a regression (just check how small is the change I needed to make to re-enable option 3). However, deprecating some of these is also an option. @brandenchan what do you think? |
So I see that option 2. is a needed feature so we should definitely keep that in code and document it. We also need a global Option 1. should be a byproduct of having the global debug so I imagine it will always work, but I also think that we don't have to document this. |
Ok, I agree option 2 is necessary as it is and option 1 is a byproduct of 2. Between 3 and 4, what do you prefer? I have no preference to be honest. Keeping both is not an issue code-wise, but I will disable either of them for clarity if we decide that it's better that way 🙂 |
If we decide to deprecate one of the options then I'd argue for deprecating option 4 ( |
That way, |
Sure I can get behind this though currently I don't feel a strong need to deprecate one. |
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
Problem
Passing the debug parameter to a pipeline as a global arg (like
p.run(query="Why?", params={"debug": True})
) did not work, asdebug
was not included in the set of valid global parameters which was populated from node signatures.Solution
Add debug in the valid global parameters set in
Pipeline.run()