Skip to content

Move install to separate step and give more leeway to benchmarks - #712

Merged
hansott merged 1 commit into
mainfrom
benchmarks
Sep 4, 2026
Merged

hansott merged 1 commit into
mainfrom
benchmarks

Conversation

@hansott

@hansott hansott commented Sep 4, 2026

Copy link
Copy Markdown
Member

The install step is executed per retry of the benchmark (up to 3 times)

More leeway because of GitHub actions runners being slow and unreliable for testing performance.

The install step is executed per retry of the benchmark (up to 3 times)

More leeway because of GitHub actions runners being slow and unreliable
for testing performance.
Comment thread sample-apps/common.mk
# ignore poetry install failures, these can be due to locking issues when
# 2 poetry installs are run at the same time.
poetry install || true;
poetry install

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Medium - Removing the install failure suppression re-breaks concurrent sample-app startup in end-to-end CI

The shared install target now returns the raw poetry install exit status even though the repository still starts two targets from the same sample-app directory in parallel during end-to-end tests. In .github/workflows/end2end.yml, make runZenDisabled is backgrounded and then make run is started shortly after; both targets depend on install, so they can hit the same Poetry lock/contention that the removed || true was explicitly masking. When one of those installs exits non-zero, the corresponding app process never comes up and the matrix job fails or flakes across many sample apps.

Suggested change
poetry install
poetry install || true

More info - Reply on this comment to give feedback or ignore the issue.

@hansott
hansott merged commit acff21e into main Sep 4, 2026
124 of 125 checks passed
@hansott
hansott deleted the benchmarks branch September 4, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants