build: harden uv workspace configuration - #1768
Merged
Merged
Conversation
Fixes several ways the uv setup was already broken or could break silently. Workspace members are now listed explicitly instead of matching `packages/*`. Any stray directory under packages/ -- tool output, scratch, build leftovers -- matched that glob and, having no pyproject.toml, broke every uv command in the repo with "missing a pyproject.toml". The Docker image installs the tox group from uv.lock via `uv export` instead of `uv pip install --group`, which re-resolves fresh from the index and ignores the lock. That re-resolution is why the tox group carried its own `uv~=0.12.0` pin, duplicating [tool.uv] required-version; the pin is removed, so the export and the removal must stay in the same commit. Lock hashes are no longer stripped, so the install is hash-verified at no measurable cost. `constraint-dependencies` now bounds the locked uv. tox-uv depends on the uv PyPI package with no upper bound, and the image takes uv's version straight from the lock, so a plain `uv lock --upgrade` could have put uv 0.13.x in the image and tripped required-version at runtime -- the same failure the removed pin guarded, reached by a different route. UV_PROJECT_ENVIRONMENT redirects the in-container project environment. The repo is bind-mounted at /data, so `uv run` inside the test container was rebuilding the developer's host .venv against the container's Linux interpreter, leaving the host with a venv pointing at a non-existent python. UV_LOCKED additionally stops any in-container uv command rewriting the mounted uv.lock. `TOX ?= uv run tox` in project_common.mk, overridden to bare `tox` in the image, so container runs use the lock-pinned system tox already installed there instead of first syncing the whole workspace into a throwaway env to obtain the same tox (measured: 58 packages, ~7s per container). default-groups covers dev/lint/type/test/tox so a bare `uv sync` produces a venv that can run every make target. Previously only `dev` was installed, so lint/format/type-check failed outright and `make test` silently fell through to whatever tox was on PATH -- a system tox without the tox-uv plugin then dies on "runner 'uv-venv-lock-runner' is not available". Accepted cost: the three CI steps that use `uv sync --group ...` widen from two groups to five, about 9 packages and 86MB per job; the `--only-group` steps are unaffected. The root test group gains jsonschema and drops the version bounds that each member's own test group already declares. Without jsonschema, gooddata-flexconnect's tests could not even be collected from the root venv, which is the environment scripts/validate_python.sh uses for filtered runs. gooddata-pipelines' test group loses its Poetry-style specifier syntax while keeping its major-version ceilings. The staging make targets use `uv run --locked python` rather than bare `python`; the staging workflow syncs .venv but never activates it, so those two scripts ran without orjson, pyyaml or requests. Dead `wheel_build_env` is removed from the eight package tox.ini files. tox itself reported it as an unused key: uv-venv-lock-runner installs from the lock and never builds a package env, and no .pkg env is created in practice. pre-commit's ruff is pinned to v0.15.20 to match the lint group, which was already resolving to 0.15.20 while the hook ran 0.15.1. jira: trivial risk: low
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (3)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthroughThe pull request configures an explicit uv workspace, aligns test dependencies, and updates tox execution for locked environments. Docker, Make targets, and package tox files use the new execution model. The Ruff pre-commit hook and lint workflow are updated. ChangesWorkspace and test tooling
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to This build and tooling configuration change has no actionable merge-blocking risk remaining and is merge-ready after normal checks and review. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1768 +/- ##
==========================================
- Coverage 80.83% 80.82% -0.01%
==========================================
Files 272 272
Lines 19416 19414 -2
==========================================
- Hits 15694 15692 -2
Misses 3722 3722 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
tox-uv is a thin wrapper: the plugin itself lives in tox-uv-bare, and the only thing the wrapper adds is a dependency on the `uv` PyPI package. That package is a 58MB binary the test image already has (COPYed from ghcr.io/astral-sh/uv) and the host already has on PATH, so installing it again bought nothing except an overwrite of the COPYed binary. Dropping it removes two distributions from the lock and shrinks each test image from 378MB to 302MB. `tox --version` still reports the plugin registered, since tox-uv-bare was always the thing providing it. Because the wrapper's `uv` dependency is gone, [tool.uv] constraint-dependencies has nothing left to constrain and is removed. That also inverts the earlier argument for leaving the base image tag floating: the COPY is now the image's only source of uv rather than something the lock overwrote, so it is pinned to an exact 0.12.5 instead of the 0.12 tag. Also adds a .gitignore to gooddata-eval, the only package without one. Its sibling packages all ignore .tox/, .coverage, coverage.xml and the json report; without them, `make test` in that package leaves the artifacts staged for an accidental commit. jira: trivial risk: low
The job named `lint-and-format-check` only ever ran `make format`, so ruff's linter was never enforced anywhere except pre-commit -- which runs on changed files only. Three violations reached master as a result, and `make lint` has been failing on a clean checkout. Fixes all three, then adds the missing step so the job does what its name says: - sse_client.py `_is_retryable_exc` ended with an `if isinstance(...): return True` / `return False` pair (SIM103). Collapsed to returning the isinstance directly; the comment explaining why RemoteProtocolError is retryable moves above the return and is unchanged. - test_agentic_runner.py imported `AGENTIC_TEST_KINDS` and `AgenticEvalOutcome` inside two test bodies (PLC0415, twice). Both names were already imported at module top -- `AgenticEvalOutcome` identically, `AGENTIC_TEST_KINDS` from a module the file already imports from -- so the local imports were redundant, not deliberate deferrals. Removed, and AGENTIC_TEST_KINDS added to the existing top-level import. No behaviour change: the isinstance chain returns the same value for every input, and the moved imports resolve to the same objects. jira: trivial risk: low
no23reason
approved these changes
Sep 1, 2026
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
2 participants
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.
Hardens the
uvworkspace configuration. Build/tooling config only — no Python source changed.Two things that were actively broken
members = ["packages/*"]was a booby trap. Any stray directory underpackages/matched the glob and, having nopyproject.toml, broke every uv command in the repo:Members are now listed explicitly. A new package needs three entries —
members,[project].dependencies, and[tool.uv.sources]— and omitting this one fails loudly during resolution rather than silently.make test-ciwas destroying the developer's host venv. The repo is bind-mounted at/datain the test container, souv runinside resolved the project environment to the host's.venvand rebuilt it against the container's Linux interpreter:Fixed with
ENV UV_PROJECT_ENVIRONMENT=/tmp/uv-project-venv, plusENV UV_LOCKED=1so no in-container uv command can rewrite the mounteduv.lock.Determinism
The image installed the tox group with
uv pip install --group tox, which re-resolves fresh from the index and ignoresuv.lock. That is why thetoxgroup carried its ownuv~=0.12.0pin, duplicating[tool.uv] required-version. Now it exports from the lock instead, and lock hashes are kept so the install is hash-verified:The duplicated pin is gone, replaced by
constraint-dependencies = ["uv~=0.12.0"].tox-uvdepends on theuvpackage with no upper bound and the image now takes uv's version from the lock, so a plainuv lock --upgradecould otherwise have put uv 0.13.x in the image and trippedrequired-version— the same failure the old pin guarded, reached by a different route.Ergonomics
default-groups = ["dev", "lint", "type", "test", "tox"]. Previously a bareuv syncinstalled onlydev, somake lint/format/type-checkfailed outright, andmake testsilently fell through to whatevertoxwas onPATH— a system tox without thetox-uvplugin dies onrunner 'uv-venv-lock-runner' is not available.TOX ?= uv run toxinproject_common.mk, overridden to baretoxin the image, so container runs use the lock-pinned system tox already installed there rather than first syncing the whole workspace into a throwaway env to obtain the same tox (measured: 58 packages, ~7s per container).The root
testgroup gainsjsonschemaand drops the version bounds its members already declare. Withoutjsonschema,gooddata-flexconnect's tests could not even be collected from the root venv — the environmentscripts/validate_python.shuses for--test-filterruns. Before/after in that package:ModuleNotFoundError→ 61 passed.The staging targets use
uv run --locked pythoninstead of barepython;staging-tests.yamlsyncs.venvbut never activates it, so those two scripts ran withoutorjson,pyyamlorrequests.Cleanup
wheel_build_envremoved from the 8 packagetox.inifiles. tox reported it as an unused key —uv-venv-lock-runnerinstalls from the lock and never builds a package env, and no.pkgenv is created in practice.gooddata-pipelines' test group loses its Poetry-style specifier syntax while keeping its major-version ceilings.v0.15.20, matching thelintgroup that already resolved to 0.15.20 while the hook ran 0.15.1.Accepted cost
default-groupswidens the three CI steps that useuv sync --group ...from two groups to five — 9 packages, ~86 MB per job (rw-python-tests.yaml:59,73,staging-tests.yaml:61). The--only-groupsteps are unaffected. Narrowing them with--no-default-groupsis a pessimization on its own: themaketarget that follows each step callsuv run, which re-widens the environment, so you get two install passes instead of one. Making it genuinely narrower requires switchingTY = uv run tyandmake test-docs-scriptsto direct.venv/bin/...invocations — a separate decision about the tool-invocation convention.uv.lockresolved versions are unchanged except forjsonschemaand its three transitive deps; every otherversion =line is byte-identical to master.Smaller image
tox-uvis a thin wrapper: the plugin lives intox-uv-bare, and the only thing the wrapper adds is a dependency on theuvPyPI package — a 58MB binary the image already has (COPYed fromghcr.io/astral-sh/uv) and the host already has on PATH. Installing it again bought nothing except an overwrite of the COPYed binary.Test image: 378MB → 302MB, 13 packages installed instead of 15.
tox --versionstill reports the plugin registered, sincetox-uv-barewas always what provided it.Two knock-on effects:
constraint-dependenciesnow has nothing to constrain (uvis gone from the lock entirely) and is removed; and theCOPYis now the image's only source of uv rather than something the lock overwrote, so the base image is pinned to exact0.12.5instead of the floating0.12tag.Lint is now actually enforced
The job named
lint-and-format-checkonly ever ranmake format. Ruff's linter was never enforced in CI — only by pre-commit, which runs on changed files. Three violations reached master as a result, andmake lintfailed on a clean checkout:Fixed, then the missing
make lintstep added so the job does what its name says.Both
PLC0415sites imported symbols already imported at module top —AgenticEvalOutcomeidentically,AGENTIC_TEST_KINDSfrom a module the file already imports from — so they were redundant, not deliberate deferrals for a cycle or an expensive import.SIM103collapsed anif isinstance(...): return True/return Falsepair into returning theisinstancedirectly, keeping the comment about the 1–4% flake rate. No behaviour change: the chain returns the same value for every input, and the moved imports resolve to the same objects.This pairs with
default-groupsabove: a bareuv syncnow installs ruff, somake lintworks locally without--all-groups.Also
gooddata-evalwas the only package with no.gitignore, somake testthere left.coverage,coverage.xml,.tox/and the json report commit-ready. Added one mirroring its siblings.Follow-ups, deliberately not in this PR
make docsis broken for all 4 packages that define it.gooddata-flexconnectandgooddata-flight-serverhave nodocs/directory at all;gooddata-fdwandgooddata-pandashave[testenv:docs]blocks whosedepsandskip_installare dead keys underuv-venv-lock-runner(tox config -e docs→# !!! unused: deps, skip_install), sosphinx-buildis never installed — and theirdocs/requirements.txtreferences unresolvable-e ../gooddata-sdkpaths and pinsSphinx~=5.1.1underbasepython = python3.14. Left entirely alone here rather than half-cleaned.gooddata-api-clientpackaging — separate plan; it still has nopyproject.tomland publishesRequires-Python: >=3.6.uv sync --group ...steps could be narrowed, but not with--no-default-groupsalone — see Accepted cost above. Doing it properly means switchingTY = uv run tyandmake test-docs-scriptsto direct.venv/bin/...invocations, which is a decision about the tool-invocation convention rather than a tweak.Investigated and deliberately not changed
.toxlooked like it had the same bind-mount problemUV_PROJECT_ENVIRONMENTfixes for.venv— container runs leave Linux venvs inpackages/*/.tox/. It doesn't. Tested directly: a.tox/py314created by a container (home = /usr/local/bin, Linux 3.14.3) then used by a host run → tox detected the foreign interpreter, transparently recreated the env (home = /opt/homebrew/opt/python@3.14/bin, 3.14.6), 8 passed; a second run's setup dropped 1.35s → 0.77s, so reuse still works. Categorically unlike the.venvbug, where uv left the host venv pointing at a dead interpreter with no recovery.TOX_WORK_DIRwould have cost.toxreuse to fix nothing.Verification
uv lock --checkmake formatmake lintmake type-checkmake test-docs-scriptsmake test— gooddata-sdk, py314, recreatedmake test— gooddata-eval, py314, recreatedmake test— gooddata-pipelines, py314, recreatedmake test-ci— gooddata-dbt, py314 and py310 containersuv 0.12.5,tox-uv-bare 1.35.2— exactly as locked.venvafter container runspre-commit run --files <changed>uv run pytestfrompackages/gooddata-flexconnectModuleNotFoundError→ 61 passedrm -rf .venv && uv sync --only-group lint --locked→make format→make lint)py310 was built and run specifically because the exported tox requirements carry
python_full_version < '3.11'markers (tomli,typing-extensions): the py310 image installs a different set than py314.uv.lockresolved versions are unchanged from master except:jsonschema+ 3 transitive deps added, andtox-uv+uvremoved.Summary by CodeRabbit