Skip to content

feat: increase Mermaid timeout and make it configurable#8973

Merged
anakin87 merged 3 commits intomainfrom
pipeline-draw-timeout
Mar 5, 2025
Merged

feat: increase Mermaid timeout and make it configurable#8973
anakin87 merged 3 commits intomainfrom
pipeline-draw-timeout

Conversation

@anakin87
Copy link
Copy Markdown
Member

@anakin87 anakin87 commented Mar 5, 2025

Related Issues

Proposed Changes:

  • increase default timeout to 30 seconds (instead of 10)
  • expose the timeout parameter in Pipeline.draw and Pipeline.show, so that users can control this behavior.

How did you test it?

CI; new unit test.
I also triggered e2e tests that were failing due to the timeout. -> https://github.com/deepset-ai/haystack/actions/runs/13673782419/job/38229648241?pr=8973

Notes for the reviewer

I think that waiting more than 10 seconds to get a Pipeline rendering is not nice.
I have the feeling that this might be related to the fact that we are now sending compressed data to the server (introduced in #8767) and decompression can also take time on the server. However, there have also been several recent changes in mermaid.ink that may have an impact on rendering times.
In any case, we should monitor what happens in the future.

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I 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 documented my code
  • I ran pre-commit hooks and fixed any issue

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Mar 5, 2025

Pull Request Test Coverage Report for Build 13674033932

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 52 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-0.2%) to 90.066%

Files with Coverage Reduction New Missed Lines %
core/pipeline/draw.py 2 87.29%
components/retrievers/in_memory/bm25_retriever.py 9 79.63%
components/retrievers/in_memory/embedding_retriever.py 11 76.27%
document_stores/in_memory/document_store.py 12 96.14%
core/pipeline/base.py 18 90.85%
Totals Coverage Status
Change from base Build 13673609441: -0.2%
Covered Lines: 9620
Relevant Lines: 10681

💛 - Coveralls

@anakin87 anakin87 marked this pull request as ready for review March 5, 2025 10:35
@anakin87 anakin87 requested review from a team as code owners March 5, 2025 10:35
@anakin87 anakin87 requested review from dfokina and julian-risch and removed request for a team March 5, 2025 10:35
Copy link
Copy Markdown
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@anakin87 anakin87 enabled auto-merge (squash) March 5, 2025 10:45
@anakin87 anakin87 merged commit bb0e36f into main Mar 5, 2025
18 checks passed
@anakin87 anakin87 deleted the pipeline-draw-timeout branch March 5, 2025 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pipeline drawing: expose timeout parameter and increase the default

3 participants