fix(ci): build the API image from the repository root - #70
Merged
Conversation
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
The failure
The
ghcrjob was the only leg of run33198050069to fail on thev0.1.0release. All three packages reached PyPI, the tag, the GitHub Release and the versioned docs all succeeded; only the image build did not:Cause: the build context does not match the Dockerfile's COPY paths
packages/api/Dockerfileinstalls the three packages from their local sources and writes its COPY paths relative to the repository root:COPYresolves against the build context, and the workflow setcontext: packages/api. So buildx looked forpackages/api/packages/adapter-sdk, which does not exist — hence "not found" for all three.The Dockerfile was changed in Task 13 to install from local paths (nothing was on PyPI yet); the workflow's context was never updated to match. The demo stack has had this right all along —
packages/nl2sql/src/nl2sql/cli/demo/schemas.pygeneratescontext: ..withdockerfile: packages/api/Dockerfile. The workflow was the outlier.The fix points the context at the repository root and names the Dockerfile explicitly (both are documented inputs of
docker/build-push-action@v6):The stale comment above the job — which claimed the Dockerfile installs from PyPI — is corrected.
needs: pypiis kept deliberately, but for a different reason than before: the image build no longer depends on PyPI, so the dependency now only ensures the image is published for a release that actually reached PyPI.The
.dockerignoreis new, and whyThere was no
.dockerignoreat the repo root or inpackages/api. Withcontext: ., the entire working tree is sent to the daemon. On CI the checkout is clean so it would have worked, but it is wasteful there and slow-to-broken on a developer machine, where the tree includes.venv-dev/(857 MB),chroma_db/(94 MB),site/,data/,logs/,.git/,__pycache__/and stalebuild/,dist/and*.egg-infodirectories — 993 MB in total on this checkout.The ignore list was chosen against what the build actually needs. Only
packages/adapter-sdk,packages/nl2sqlandpackages/apiare COPYed, so of the three manifests:packages/nl2sql/pyproject.tomldeclaresreadme = "README.md", andpackages/nl2sql/README.mdmust survive — a missing readme fails the wheel build. It is not excluded, and the built image'snl2sql-enginemetadata carriesDescription-Content-Type: text/markdownwith the README body, which proves it was picked up.packages/adapter-sdk/pyproject.tomldeclares noreadmeand the package has noREADME.md.packages/api/pyproject.tomldeclares noreadme(it has aREADME.md, but nothing references it).Patterns are matched against the path relative to the context root, so repo-level directories are named directly and anything that also appears inside a package (
build,dist,*.egg-info,__pycache__,.pytest_cache) is prefixed with**/.data/is excluded safely — the demo stack bind-mounts it at run time, it is never copied at build time.Local build evidence
CI cannot verify this: no workflow builds the image on a PR. The verification is a local build from a clean context.
BUILD_EXIT=0). Same command failed with the "not found" error before, using the old context.890.43 kB(plus a 1.06 kB.dockerignore), down from a 993 MB working tree.Processing ./packages/adapter-sdk,Processing ./packages/nl2sql,Processing ./packages/api, thenBuilding wheel for nl2sql-adapter-sdk / nl2sql-engine / nl2sql-api (pyproject.toml). There is noDownloading nl2sql...line anywhere.nl2sql-apiandnl2sql-adapter-sdkare also 0.1.0. The[postgres,mysql]extras resolved (import psycopg2, pymysqlsucceeds) andnl2sql --helpruns. Starting the container reaches uvicorn and application startup, then stops at datasource configuration, which is expected — the image needsconfigs/and live databases mounted at run time, exactly as the demo stack provides.Nothing was pushed, dispatched, re-run or tagged.
Docs
docs/development/releasing.md— theghcrstep described the Dockerfile as installing from PyPI. Corrected, with the context requirement and the reasonneeds: pypistays.docs/getting_started/docker.md— already told readers to build from the repo root; it now says why, and describes the new.dockerignore.docs/deployment/docker.mdneeded no change: it covers deploying the service, not building the image.mkdocs build --strictis clean, from a throwaway venv built fromrequirements-docs.txt.Baselines
Unchanged: unit
231 passed, 1 skipped, 47 deselected; key-free integration28 passed.Out of scope, reported not fixed
packages/api/Dockerfile.devis genuinely broken and is not touched here. Ten of its twenty COPY lines reference paths deleted in the Task 10 packaging collapse —packages/core,packages/adapter-sqlalchemyandpackages/adapters/{mssql,mysql,postgres,sqlite}— and a build fails the same way this bug did:Nothing references it: no workflow, no compose file, no doc, and no code path. It should be deleted, but that is a separate decision and a separate PR.