[None][infra] Bake pre-commit hook cache into tritondevel CI image - #17530
Closed
brnguyen2 wants to merge 1 commit into
Closed
[None][infra] Bake pre-commit hook cache into tritondevel CI image#17530brnguyen2 wants to merge 1 commit into
brnguyen2 wants to merge 1 commit into
Conversation
Pre-populate the pre-commit hook store at image-build time so the CI type-check stage no longer clones hooks from github.com at test time. pre-commit enumerates every remote repo in .pre-commit-config.yaml (cloning each to read its hook manifest) before running the selected hook, so a single 'git fetch origin --tags' against github.com can fail the whole 'Run type check' stage on test nodes without reliable github access. Build nodes do reach github (see install_mooncake.sh / install_ucx.sh), so cloning the hooks once at build time leaves the runtime store warm and offline. - add docker/common/install_precommit_hooks.sh: seeds a throwaway git repo with the config and runs 'pre-commit install-hooks' into a fixed PRE_COMMIT_HOME - tritondevel stage: set ENV PRE_COMMIT_HOME=/opt/pre-commit-cache and invoke the script (bind-mounting .pre-commit-config.yaml) Cache is keyed by (repo, rev); a PR that bumps a hook rev misses the cache until the dep image is rebuilt, falling back to the current github-clone behavior for that PR only. CI-only: the cache lives in tritondevel, not in the shipped release image, so no ATTRIBUTIONS change. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>
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.
Description
Pre-populate the pre-commit hook store in the
tritondevelCI image at build time, so theRun type checkstage no longer clones hooks from github.com at test time.Problem. The type-check stage runs
python3 -m pre_commit run type-check --all-files(jenkins/L0_Test.groovy). Even thoughtype-checkis arepo: localhook that touches no network, pre-commit enumerates every remote repo in.pre-commit-config.yaml— it clones each one (git fetch origin --tags) to read its hook manifest — before it runs the selected hook. So a single failed clone of e.g.github.com/pycqa/isortfails the whole stage:This is transient infra flakiness on test nodes that lack reliable github access, not a real type-check failure.
Fix. Build nodes do reach github anonymously (see
install_mooncake.sh,install_ucx.sh,install_nixl.sh, which alreadygit clone https://github.com/...at build time). Cloning the hooks once at build leaves the runtime store warm and fully offline.docker/common/install_precommit_hooks.sh(new): seeds a throwaway git repo with.pre-commit-config.yamland runspre-commit install-hooksinto a fixedPRE_COMMIT_HOME, cloning every hook repo and building every hook env.docker/Dockerfile.multi(tritondevelstage):ENV PRE_COMMIT_HOME=/opt/pre-commit-cache+ aRUNthat bind-mounts the config and invokes the script.Design notes.
PRE_COMMIT_HOME(not the default~/.cache/pre-commit): hook virtualenvs bake absolute shebang paths, so build-time and runtime paths must be identical. Set once as anENVso runtimepre_commitreads the same store.tritondevelonly, notdevel: scopes the cache to the CI image, keeping it out of the shippedreleaseimage (so no ATTRIBUTIONS change — nothing new is redistributed).pip installed fromrequirements-dev.txtat test time; the build-time install only populates the store.Caveat. The store is keyed by (repo, rev). A PR that bumps a hook
rev:misses the cache for that repo and falls back to the current github-clone behavior for that PR only, until the dep image is rebuilt. A follow-up can add a build-timegit config --global url.<internal-mirror>.insteadOf https://github.com/<owner>/to close even that gap.Test Coverage
Mechanics verified locally against pre-commit 4.6.2: after
install-hookspopulatesPRE_COMMIT_HOME, a subsequentpre-commit runreuses the cache with zero network (no "Initializing environment" line). End-to-end validation requires atritondevelimage rebuild (theBuild-Docker-Imageshelper job) and ajenkins/current_image_tags.propertiesrepin; the resultingRun type checkstage should show no "Initializing environment for https://github.com" line.PR Checklist