Skip to content

[ML] Make build timing regression check a soft failure#3004

Open
edsavage wants to merge 3 commits intoelastic:mainfrom
edsavage:fix/soft-fail-regression-check
Open

[ML] Make build timing regression check a soft failure#3004
edsavage wants to merge 3 commits intoelastic:mainfrom
edsavage:fix/soft-fail-regression-check

Conversation

@edsavage
Copy link
Contributor

@edsavage edsavage commented Mar 19, 2026

Summary

  • The "Check build timing regressions" step should be purely informational and never cause the overall build to fail. The generated analytics step already had soft_fail: true, but the upload step that creates it did not — so errors during pipeline upload (e.g. missing step dependencies) would still produce a hard failure.
  • Add an optional soft_fail parameter to generate_step() and use it for the regression check step, making both the upload and the analytics step soft-fail on any error.

Test plan

  • Verified the generated pipeline JSON includes "soft_fail": true on the upload step
  • CI passes on this PR

Labeling as >non-issue as this change is purely internal.

Made with Cursor

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes the “Check build timing regressions” Buildkite step purely informational by ensuring the pipeline upload step itself is also soft-failing, matching the already-soft-failing analytics step it generates.

Changes:

  • Add an optional soft_fail parameter to PipelineStep.generate_step().
  • Use soft_fail=True for the “Check build timing regressions” pipeline upload step so it never fails the overall build.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
.buildkite/pipeline.json.py Marks the build timing regression check upload step as soft_fail=True.
.buildkite/ml_pipeline/step.py Extends generate_step() to optionally emit "soft_fail": true on the generated Buildkite step.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@prodsecmachine
Copy link

prodsecmachine commented Mar 19, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

The "Check build timing regressions" pipeline step should never cause
the overall build to be marked as failed.  The generated analytics step
already had soft_fail: true, but the upload step that creates it did
not — so errors during pipeline upload (e.g. missing step dependencies)
would still produce a hard failure.

Add a soft_fail parameter to generate_step() and use it for the
regression check, making both the upload and the analytics step
soft-fail on any error.

Made-with: Cursor
…lable

The run-validation.cmake script supports an OPTIONAL flag to skip
gracefully when Python 3 is not on the PATH. Pass it from the CI
test runner so that environments without Python (e.g. the Linux
Docker build image) don't fail the entire test step.

Made-with: Cursor
@edsavage edsavage force-pushed the fix/soft-fail-regression-check branch from efb7477 to f3fda76 Compare March 23, 2026 03:02
Skip the allowlist validation block if cmake/run-validation.cmake
does not exist, so this change remains safe even if the graph
validation feature is reverted independently.

Made-with: Cursor
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.

3 participants