Add support for monorepos with shared local dependencies - #67
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Three critical findings remain unresolved in the workflow.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds uv workspace support to the reusable Python build workflow for monorepos and cross-platform Lambda packaging.
Changes:
- Adds workspace and target-platform inputs.
- Builds workspace wheels and installs local and locked dependencies.
- Packages artifacts for the selected platform.
File summaries
| File | Review |
|---|---|
.github/workflows/build.python.yml |
Three critical issues remain: wheel names are not fully normalized, Python versions below 3.11 cannot use the current tomllib discovery, and missing wheels are not treated as fatal. |
Review details
Suppressed comments (1)
.github/workflows/build.python.yml:129
uv exportemits the selected root workspace project as-e .. This pattern only matches-e ./<non-empty-path>, so a valid call withworking-directory: "."never adds the application wheel toLOCAL_WHEELS; the artifact then lacks the project itself (and may invokeuv pip installwith no package arguments). Include the-e .form when extracting local paths.
done < <(grep -oE '^-e \./\S+' /tmp/uv-workspace-full-export.txt | sed 's/^-e //')
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate packaging and compatibility issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
.github/workflows/build.python.yml:101
python_versionis caller-selectable, but this workspace path unconditionally importstomllib, which is only available in Python 3.11+. A caller using Python 3.10 or older will fail before packaging (the second metadata read below has the same problem). Either enforce Python >=3.11 for this mode or use a compatible TOML parser for both reads.
PKG_NAME="$(python3 -c "import tomllib; print(tomllib.load(open('${WORKING_DIRECTORY}/pyproject.toml','rb'))['project']['name'])")"
.github/workflows/build.python.yml:113
- This file is used as a constraints file, but
--no-emit-workspacedoes not exclude the package selected by--package; the selected project is still emitted as an editable local requirement by default. Passing that-eentry through-ccan makeuv pip installreject the constraints file or resolve the target from its source instead of the wheel. Exclude the selected project as well so this file contains only third-party constraints.
uv export --package "$PKG_NAME" --no-emit-workspace --no-dev --no-hashes -o /tmp/uv-workspace-constraints.txt
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Address the two moderate workflow issues before approval.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
.github/workflows/build.python.yml:101
- When this workflow is called with a Python version below 3.11, both
tomllibinvocations in this step fail withModuleNotFoundError, even though the input accepts any Python version. Add the agreed upfront version check (or use a compatible TOML parser) so the workflow reports a clear configuration error before attempting the workspace build.
.github/workflows/build.python.yml:133
- The export can emit valid workspace entries such as
-e .or a relative path without the./prefix. This extracts the path but never normalizes it relative to the checkout (and does not handle afile://URL), so the subsequentopen()/findlookups can fail or skip the root workspace package. Parse the editable target and convert it to a checkout-relative filesystem path before reading its metadata and locating its wheel.
done < <(awk '$1 == "-e" { print $2 }' /tmp/uv-workspace-full-export.txt)
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Workspace file URLs must be converted to filesystem paths before reading package metadata.
Review details
Suppressed comments (1)
.github/workflows/build.python.yml:128
uv exportwrites editable workspace entries as file URLs (for example,-e file:///.../libs/shared), not filesystem paths. The value extracted here is then used directly inopen('${local_dir}/pyproject.toml'), so workspace builds will try to openfile:///.../pyproject.tomlas a literal relative filename and fail before installing any wheels. Convert file URLs to paths (or derive the workspace member paths from the repository root) before readingpyproject.toml.
local_name="$(python3 -c "import tomllib; print(tomllib.load(open('${local_dir}/pyproject.toml','rb'))['project']['name'])")"
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Workspace file:// URIs must be normalized before discovery can read pyproject.toml.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
|
|
||
| # Build every workspace member once, so local (path) dependencies of | ||
| # this package are available as real wheels alongside it. | ||
| uv build --all-packages --wheel |
There was a problem hiding this comment.
Kan være lurt å legge til --frozen/--locked her. Uten det kan uv stille re-resolve og avvike fra uv.lock i CI hvis pyproject.toml/lock er ute av sync, i stedet for å feile tydelig.
Gjelder også linje 113 og 118
| uv build --all-packages --wheel | |
| uv build --all-packages --wheel --frozen |
OBS: Dette er en god miks av "vibing" og selvskrevet kode, jeg er åpen for å diskutere disse endringene sammen og om vi kan kjøre en annen løsning for vår tjeneste. Målet mitt er å få våre lambdaer over på felles platform-actions og oppgradere terraform lambda-modulen til 2.3.0. Vi kan også skrive om egen arkitektur hvis det er bedre.
PRen legger til støtte for uv-workspaces. Motivasjonen er fra PR i alternative-transport-realtime-processing.
Jeg har forsøkt å migrere repoet over på ny versjon av lambda-modulen (2.3.0), men slik jeg har forstått det krever det også at man bruker nye pipelines fra platform-actions. Vårt repo er bygget opp annerledes enn f.eks team-management-service der hver lambda har sin egen terraform-mappe og egne avhengigheter.
Vi har en terraform-mappe med hver lambda som en modul i den delte infrastruktur-mappen. I tillegg har vi delte fellesbiblioteker i repoet som begge lambdaene benytter seg av (
libs/*).Vår nåværende
build-and-deploy.ymlbygger slik:discovered-services er de to lambdaene vi har. Jeg har noe begrenset kunnskap om uv, pyproject og wheels, men siden vi bruker ARM er vi avhengig av riktig plattform-type ref AWS Python Platform
Jeg er åpen for andre forslag da denne løsningen kanskje er litt vel skreddersydd hvordan vi gjør det.