Skip to content
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

Rename Pipeline.__init__() argument max_loops_allowed as it is misleading #8291

Closed
silvanocerza opened this issue Aug 27, 2024 · 1 comment · Fixed by #8354
Closed

Rename Pipeline.__init__() argument max_loops_allowed as it is misleading #8291

silvanocerza opened this issue Aug 27, 2024 · 1 comment · Fixed by #8354
Assignees
Labels
breaking change P2 Medium priority, add to the next sprint if no P1 available type:enhancement

Comments

@silvanocerza
Copy link
Contributor

silvanocerza commented Aug 27, 2024

Summary and motivation

The name max_loops_allowed is misleading when calling Pipeline.__init__() as it doesn't limit the number of times a cycle in the Pipeline graph is executed.

Instead it limits the number of times a single Component can run. This is misleading for the user and we should rename it to something more sensible. max_run_per_component could be a good alternative.

We should deprecate max_loops_allowed to start with.

@julian-risch
Copy link
Member

I agree that the name is misleading. At least the docstring we have explains it better.
:param max_loops_allowed: How many times the pipeline can run the same node before throwing an exception.

Isn't max_loops_allowed still functioning as an upper limit for the number of times a cycle in the Pipeline graph is executed?
You're saying

it doesn't limit the number of times a cycle in the Pipeline graph is executed.

Yet Haystack won't execute a cycle more often than max_loops_allowed right?

@julian-risch julian-risch added the P2 Medium priority, add to the next sprint if no P1 available label Aug 28, 2024
@silvanocerza silvanocerza changed the title Rename Pipeline.run() argument max_loops_allowed as it is misleading Rename Pipeline.__init__() argument max_loops_allowed as it is misleading Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change P2 Medium priority, add to the next sprint if no P1 available type:enhancement
Projects
None yet
2 participants