Skip to content

fix: In AsyncPipeline cancel inflight tasks if a PipelineRuntimeError is raised by another component#11499

Merged
sjrl merged 4 commits into
v3from
cancel-inflight-tasks-async-pipeline
Jun 5, 2026
Merged

fix: In AsyncPipeline cancel inflight tasks if a PipelineRuntimeError is raised by another component#11499
sjrl merged 4 commits into
v3from
cancel-inflight-tasks-async-pipeline

Conversation

@sjrl
Copy link
Copy Markdown
Contributor

@sjrl sjrl commented Jun 3, 2026

Related Issues

Proposed Changes:

Fixes an issue that any inflight tasks would leak if a component raised an error in AsyncPipeline with concurrency > 1

How did you test it?

Added new unit test

Notes for the reviewer

Found this while investigating merging Pipeline and AsyncPipeline. Was wondering since we are leaning towards merging the two classes if it would be possible to also bring concurrency to Pipeline.run. This is somewhat complicated by the BreakPoint feature though since the creation of a PipelineSnaphsot is only valid if there are no in-flight tasks.

Checklist

  • I have read the contributors guidelines and the code of conduct.
  • I have updated the related issue with new insights and changes.
  • I have added unit tests and updated the docstrings.
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I have documented my code.
  • I have added a release note file, following the contributors guidelines.
  • I have run pre-commit hooks and fixed any issue.

@sjrl sjrl requested a review from a team as a code owner June 3, 2026 12:21
@sjrl sjrl requested review from anakin87 and removed request for a team June 3, 2026 12:21
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
haystack-docs Ignored Ignored Preview Jun 5, 2026 7:05am

Request Review

@sjrl sjrl self-assigned this Jun 3, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/core/pipeline
  async_pipeline.py 535, 540, 557-561, 565-577, 618-627, 652
Project Total  

This report was generated by python-coverage-comment-action

@claude
Copy link
Copy Markdown

claude Bot commented Jun 3, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Copy link
Copy Markdown
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I left two comments.

Was wondering since we are leaning towards merging the two classes if it would be possible to also bring concurrency to Pipeline.run.

I would support this if feasible. Concurrency!=async and this would make things clearer.

Comment thread haystack/core/pipeline/async_pipeline.py
Comment thread releasenotes/notes/cancel-in-flight-tasks-on-error-8b9623a305717328.yaml Outdated
@sjrl sjrl requested a review from anakin87 June 5, 2026 06:44
Copy link
Copy Markdown
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@sjrl sjrl merged commit cde8760 into v3 Jun 5, 2026
23 of 24 checks passed
@sjrl sjrl deleted the cancel-inflight-tasks-async-pipeline branch June 5, 2026 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants