Repository navigation
ci: Let heavy cookbooks skip notebook runs; keep pins - #99
Closed
alexey-temnikov wants to merge 4 commits into
Closed
alexey-temnikov wants to merge 4 commits into
alexey-temnikov wants to merge 4 commits into
Conversation
The paths filter only listed cookbooks/**, so a PR that changes only the workflow or its pinned toolchain never triggered it; the workflow has not run on its own PR. Add both workflow files to the push and pull_request filters, and add workflow_dispatch so maintainers can run it on demand. Signed-off-by: Alexey Temnikov <attemnik@amazon.com>
The Python job installed requirements.txt and then the notebook toolchain in a second pip call. The unpinned jupyter metapackage's dependencies then upgraded the cookbook's own pins (e.g. jupyterlab 4.4.5 -> 4.6.4, ipykernel 6.30.0 -> 7.4.0), so CI did not test the environment a reader installs. Install both in one pip call so the resolver treats the cookbook's == pins as hard requirements and fits the toolchain around them; a real conflict now fails the install step instead of silently upgrading. Signed-off-by: Alexey Temnikov <attemnik@amazon.com>
A cookbook whose stack cannot fit on a GitHub-hosted runner (e.g. the kv-caching cookbook's two vLLM CPU replicas) fails the notebook step as soon as a cell talks to a service other than Valkey, so it can never go green. Honor a .ci-structural-only marker file next to the notebooks: the execute step prints its first line as a notice and skips notebook execution. Lint and, for Python, the standalone test_*.py structural tier still run. The marker is checked in every language job so the rule is the same everywhere; the default stays full execution. The cookbook path is passed through env rather than interpolated into the script. Signed-off-by: Alexey Temnikov <attemnik@amazon.com>
The standalone test_*.py step used the default success() condition, so a failing notebook hid the config and compose-wiring results. Run it unless the job was cancelled so both tiers always report. Signed-off-by: Alexey Temnikov <attemnik@amazon.com>
MatthiasHowellYopp
added a commit
that referenced
this pull request
Oct 1, 2026
Ports the structural-only opt-out from #99 onto the uv-based job: - A cookbook with a .ci-structural-only file skips notebook execution (lint and the structural test_*.py tier still run) in all four language jobs. - The workflow now also triggers on changes to its own files and via workflow_dispatch. - The Python cookbook-tests step runs even when notebook execution fails (if: !cancelled()). #99's single-pip-install pin-protection is not ported: under uv the lockfile already pins the full tree, so the toolchain can't upgrade the cookbook's deps. Signed-off-by: MatthiasHowellYopp <matthias.howell@improving.com>
Collaborator
|
Applied in d94d74a on ci-cookbook-validation, reconciled with the uv CI rewrite. I ported the .ci-structural-only opt-out across all four language jobs, the workflow-file path triggers and workflow_dispatch, and if: !cancelled() on the Python tests step. I did not port the single-pip-install pin-protection commit: that job now uses uv, and the lockfile already pins the full transitive tree, so the toolchain can't upgrade the cookbook's deps. actionlint is clean. Closing in favor of that commit. |
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.
Targets #43 (
ci-cookbook-validation).Summary
Once #76 lands, the
python-notebooksjob would discovercookbooks/kv-caching/. It would then fail on both Valkey versions withAPIConnectionErrorat the first replica call, because the notebook needs vLLM replicas that a GitHub runner can't host. This PR has one commit per fix:.ci-structural-onlyfile opts a cookbook out (X1). The job prints the file's first line as a::notice, skips executing the notebook, and still runs lint andtest_*.py. The check is in all four language jobs. The cookbook path reaches the script through an env var, not${{ }}interpolation.jupyter==1.1.1silently upgraded the cookbook's pins (jupyterlab 4.4.5 → 4.6.4, ipykernel 6.30.0 → 7.4.0). A real conflict now fails the install instead.workflow_dispatch(X3). Today it has never run on this PR.if: ${{ !cancelled() }}, so it still runs when notebook execution fails (X4).Verification
Checked on an x86 host. actionlint 1.7.7 with shellcheck 0.10.0 reports 0 errors. I also replayed the job's steps from the workflow YAML against #76's
cookbooks/kv-caching/:APIConnectionError)The marker file itself is added on #76's side, in #98.
This was a replay, not a real Actions run. GitHub service networking was not exercised.
Notes I did not change:
valkey-bundle:9.1.2image reportsvalkey_version:9.1.1, and8.1.7reports 8.1.8.::warning::for the vllm-router config is still there.pip install --upgrade pipstill floats.