fix(docker): add fastembed-cache-empty placeholder so default bake builds#409
Open
SandyChapman wants to merge 1 commit into
Open
fix(docker): add fastembed-cache-empty placeholder so default bake builds#409SandyChapman wants to merge 1 commit into
SandyChapman wants to merge 1 commit into
Conversation
…ilds docker-bake.hcl's FASTEMBED_CACHE_CONTEXT defaults to docker/fastembed-cache-empty, which Dockerfile.nmp-api COPYs (`COPY --from=fastembed-cache .`). The directory was never committed — git can't track empty dirs and there was no placeholder — so any build using the committed default (a clean checkout running `make docker-build` / `docker-load`, the documented local path) fails immediately: ERROR: failed to get build context fastembed-cache: stat docker/fastembed-cache-empty: no such file or directory CI's image jobs don't hit this because ci.yaml overrides FASTEMBED_CACHE_CONTEXT to a RUNNER_TEMP dir it creates. Commit a tracked placeholder so the default context exists on checkout; the empty cache just makes the image download the embed model at build time (the existing else-branch in Dockerfile.nmp-api). Verified: with the dir present, `make docker-load TARGET=docker-cpu` proceeds past context resolution into the image builds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Sandy Chapman <schapman@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a ChangesFastembed Cache Directory Placeholder
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
crookedstorm
approved these changes
Jun 23, 2026
crookedstorm
left a comment
Contributor
There was a problem hiding this comment.
Sorry, I failed to commit mine locally! LGTM
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.
Summary
A clean checkout cannot run the default Docker build.
docker-bake.hcldefaultsFASTEMBED_CACHE_CONTEXTtodocker/fastembed-cache-empty(added in #384), andDockerfile.nmp-apiconsumes it viaCOPY --from=fastembed-cache .. That directory was never committed — git can't track empty dirs and there was no placeholder file — so the documented local build fails before it starts:CI's "Build CPU smoke images" job never hits this because
ci.yamloverridesFASTEMBED_CACHE_CONTEXTto a${RUNNER_TEMP}/fastembed-cachedir itmkdirs — so only the committed default path (local dev,make docker-build/docker-load) is broken.Fix
Commit a tracked placeholder (
docker/fastembed-cache-empty/.gitkeep) so the default context exists on checkout. The empty context just means the image downloads the embed model at build time — the existing else-branch inDockerfile.nmp-apialready handles that.Testing
maincheckout:make docker-load TARGET=docker-cpufailed immediately with the context error above.Notes
Found while doing RCA on an unrelated
docker-buildstatus failure. This is a separate, pre-existing bug (default/local builds only); the external CIdocker-buildgate is unaffected because it overrides the cache context.🤖 Generated with Claude Code
Summary by CodeRabbit