only rebuild changed methods on PRs - #221
Merged
Merged
Conversation
every push and PR rebuilds all 27 images, because the gate in build-and-test is hardcoded to should_run=true (5b13029). check-changes already computes what changed, but nothing consumes it. one method submission can cost a lot: #209 went through 16 full 27-image runs, #210 and #212 another 24 between them. pull requests now build only the methods they touch. everything else - pushes to master/dev, the new weekly schedule, manual dispatch - still rebuilds everything, so a method that breaks from upstream drift without anyone touching it still gets caught. that drift is calendar-driven, which is what the schedule is for; during a quiet stretch there are no merges to catch it. - a method rebuilds if either algorithms/<name>/ or experiment/methods/<name>/ changed. the second one matters: a regressor.py edit has to retest the method even though the install is untouched. - changes to shared build inputs (dockerfiles, base_environment, scripts, entry.sh, configure.sh, workflows) still rebuild everything. - build-and-test always runs and always reports for every algorithm, so the check names stay present and can be marked required. only the docker build step is skipped. - dropped always() from build-and-test. with the gate inside the job, a failed check-changes would have left an empty build list, skipped every build and reported green. - check-changes no longer diffs against github.event.before, so a force-push to a CI branch no longer fails the job. also fixes a long-standing bug: changed-experiments used awk field $2 on experiment/methods/<name>/..., which is the literal string "methods", not the method name. it needs $3. nothing consumed that output before, so it never showed up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
depends on #220 - don't merge this until that lands, otherwise #220 stops being a clean promote of the docs/guardrail work.
every push and PR currently rebuilds all 27 images. the gate in build-and-test is hardcoded to
should_run=true(5b13029), and check-changes computes what changed but nothing consumes it. the cost lands on contributors iterating: #209 went through 16 full 27-image runs, #210 and #212 another 24 between them.pull requests now build only the methods they touch. everything else still rebuilds everything:
the schedule is the part that matters most here. several methods install from git or unpinned pip and break with nobody touching the repo - that's calendar-driven, so during a quiet stretch there are no merges to catch it. that's roughly what happened in late july.
details:
algorithms/<name>/orexperiment/methods/<name>/changed. the second matters: a regressor.py edit has to retest the method even though the install is untouched.always()from build-and-test. with the gate inside the job, a failed check-changes would have left an empty build list, skipped every build, and reported green.github.event.before, so a force-push to a CI branch no longer fails it. that's the red X that showed up on dev earlier today.also fixes a long-standing bug:
changed-experimentsused awk field$2onexperiment/methods/<name>/..., which is the literal stringmethods, not the method name. it needs$3. nothing consumed that output before so it never surfaced.the selection logic was checked against these cases before pushing: