feat: support python-uv build method for Lambda layers - #9113
Conversation
|
Thanks for the PR, and apologies for the long silence here. I checked this out locally and ran it against a real template. The direction is right — 1. The layer build still fails after this change
Built with this branch — minimal template, one layer with Note the 2.
|
Layers declared with BuildMethod: python-uv previously failed with UnsupportedRuntimeException since get_layer_subfolder only recognized runtime identifiers and makefile. Add python-uv to the layer subfolder map so layers can share the same beta uv-based build as functions. Also collapse the experimental-flag prompt into a single loop over distinct build methods across functions and layers, so multiple resources sharing python-uv only prompt once instead of once per resource. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolve the runtime handed to Lambda Builders for python-uv layers from CompatibleRuntimes instead of passing the BuildMethod through, both in _build_layer and in the incremental manifest hash used by --cached. Reject --use-container for uv builds with an actionable message, since the build images do not ship uv yet. Abort the build when the beta prompt is declined, prompt in deterministic order, and keep the JSON output guard. Add unit and integration coverage for the uv layer path. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
559ad35 to
1f0f85b
Compare
| f"BuildMethod '{build_method}' for layers requires CompatibleRuntimes to be set, " | ||
| "so the Python version to build for can be determined" | ||
| ) | ||
| return compatible_runtimes[0] |
There was a problem hiding this comment.
[INPUT_VALIDATION] resolve_layer_build_runtime returns compatible_runtimes[0] without checking that the entry is actually a Python runtime, and without telling the user which one it picked.
Two concrete consequences:
- A template like
Metadata:
BuildMethod: python-uv
Properties:
CompatibleRuntimes:
- nodejs22.x
- python3.13hands nodejs22.x to the uv workflow, which derives --python-version from it. That fails deep inside Lambda Builders with an opaque message — exactly the class of error this function was added to prevent. Worse, on the --cached path DependencyHashGenerator resolves nodejs22.x to NODEJS_NPM_CONFIG and starts hashing package.json.
- When several Python runtimes are listed, the build silently targets the first one. Since uv installs ABI-specific wheels, a layer declared compatible with
[python3.11, python3.13]getscp311wheels and breaks at runtime on 3.13. Note that_build_layeralready warns for the analogous makefile/container case ("For container layer build, first compatible runtime is chosen as build target for container."), so silence here is inconsistent with the surrounding code.
Suggest validating the resolved value and logging the choice:
if not compatible_runtimes:
raise UnsupportedRuntimeException(...)
runtime = compatible_runtimes[0]
if not runtime.startswith("python"):
raise UnsupportedRuntimeException(
f"BuildMethod '{build_method}' requires a Python runtime in CompatibleRuntimes, "
f"but found '{runtime}'"
)
if len(compatible_runtimes) > 1:
LOG.warning("Multiple CompatibleRuntimes declared; building for %s", runtime)
return runtimeThere was a problem hiding this comment.
Fixed in f22440a. resolve_layer_build_runtime now raises UnsupportedRuntimeException naming the offending entry when the first CompatibleRuntimes value is not a Python runtime, and logs a warning with the chosen runtime when more than one is declared. Unit tests cover both paths.
| layer_definition, layer_definition.codeuri, layer_definition.build_method | ||
| layer_definition, | ||
| layer_definition.codeuri, | ||
| resolve_layer_build_runtime( |
There was a problem hiding this comment.
[BUG] Resolving the runtime here stops the 'python-uv' runtime is not supported crash, but it also silently points the incremental cache key at the wrong manifest.
DependencyHashGenerator._calculate_dependency_hash calls get_workflow_config(self._runtime, self._code_dir, self._base_dir) with no specified_workflow, so the resolved python3.13 maps to PYTHON_PIP_CONFIG and manifest_name becomes requirements.txt — not the uv manifest. For a uv layer driven by pyproject.toml/uv.lock that happens to also carry a stale requirements.txt, editing pyproject.toml or uv.lock leaves the hash unchanged, download_dependencies stays False, and sam build --cached reuses stale dependencies with no diagnostic. That is a silent-wrong-artifact failure, which is worse than the crash it replaces.
Passing the build method through so uv layers resolve to PYTHON_UV_CONFIG would be correct: its manifest_name is None, so the hash comes back None and download_dependencies is forced to True every run. That gives up the incremental speedup but never serves stale dependencies. That requires threading specified_workflow into DependencyHashGenerator; at minimum the current trade-off should be recorded here, because the integration test test_build_single_layer_python_uv_cached only exercises a requirements.txt layer and would not catch it.
The same mismatch already exists for uv functions on develop (build_definition.runtime is the real runtime), so a shared fix is worth considering rather than replicating the behaviour for layers.
There was a problem hiding this comment.
Checked this against Lambda Builders 1.67.0 and the stale-artifact scenario does not occur. detect_uv_manifest in the uv workflow checks requirements*.txt before pyproject.toml, so when both files exist the build itself installs from requirements.txt (verified locally: a layer with requirements.txt pinning six and pyproject.toml pinning attrs builds six only). Hashing requirements.txt is therefore the correct cache key for that layout. When only pyproject.toml exists, the pip config's manifest is missing, the hash is None, and dependencies are re-downloaded every run (verified: editing pyproject.toml between --cached runs picks up the new dependency). Threading the build method through would resolve to PYTHON_UV_CONFIG, whose manifest is None, and would only remove the incremental speed-up for requirements.txt-driven uv resources without changing correctness. I kept the runtime-keyed hash and documented this trade-off in _check_whether_manifest_is_changed (f22440a). Happy to revisit if Lambda Builders changes its manifest preference.
| } | ||
| build_methods.update(layer.build_method for layer in resources_to_build.layers if layer.build_method) | ||
|
|
||
| for build_method in sorted(build_methods): |
There was a problem hiding this comment.
[BUG] sorted(build_methods) can raise TypeError because the set is built from raw template values before filtering.
The layer side is guarded (if layer.build_method), but the function comprehension only guards on function.metadata being present, not on the value:
build_methods = {
function.metadata.get("BuildMethod", "") for function in resources_to_build.functions if function.metadata
}A metadata block written as BuildMethod: with no value parses to None, so a template with one such function plus any other build method yields {None, "python-uv"}, and sorted() raises TypeError: '<' not supported between instances of 'NoneType' and 'str' — an unhandled crash rather than a build error. A single-element set happens to survive because CPython never compares, which makes this easy to miss in tests.
Filtering to the experimental methods before sorting fixes it and removes the now-redundant continue:
build_methods = {
function.metadata.get("BuildMethod") for function in resources_to_build.functions if function.metadata
}
build_methods.update(layer.build_method for layer in resources_to_build.layers)
experimental_methods = sorted(bm for bm in build_methods if bm in EXPERIMENTAL_BUILD_METHODS)
for build_method in experimental_methods:
experimental_flag = EXPERIMENTAL_BUILD_METHODS[build_method]
...There was a problem hiding this comment.
Fixed in f22440a. The set is now filtered to experimental build methods before sorting, and the redundant continue is gone. Added a unit test with one function whose BuildMethod is None alongside a python-uv function.
| if not self._container_manager or config.dependency_manager != "uv": | ||
| return | ||
| raise UnsupportedBuilderException( | ||
| f"Build method '{specified_workflow}' is not supported with --use-container yet, because the SAM build " |
There was a problem hiding this comment.
[ERROR_HANDLING] The rejection message hardcodes sam build, but ApplicationBuilder is also constructed with container_manager=self._build_context.container_manager by the sync flows (samcli/lib/sync/flows/zip_function_sync_flow.py:119, layer_sync_flow.py:274, image_function_sync_flow.py:104). A user running sam sync --use-container against a uv resource is told to "Re-run sam build without --use-container", which points at the wrong command.
Dropping the command name keeps the message correct for every entry point:
raise UnsupportedBuilderException(
f"Build method '{specified_workflow}' is not supported with --use-container yet, because the SAM build "
"images do not include uv. Re-run without --use-container."
)Also worth noting: the docstring says "Fail up front", but the check runs per-resource inside _build_layer/_build_function, so with several resources other builds may already have completed before this fires. Moving the check into BuildContext alongside _check_build_method_experimental_flag would match the docstring; otherwise the wording is misleading.
There was a problem hiding this comment.
Fixed in f22440a. The message now reads "Re-run without --use-container" so it holds for sam sync as well, and both unit tests assert the command name is absent. I kept the check in ApplicationBuilder because that is the one place every entry point (build and the three sync flows) passes through; the docstring now says the check runs per resource before Lambda Builders is invoked, rather than "up front".
Address the review bot findings on the python-uv layer support: - resolve_layer_build_runtime now rejects a non-Python first CompatibleRuntimes entry with a message naming it, instead of handing e.g. nodejs22.x to the uv packager, and warns which runtime is targeted when several are declared. - _check_build_method_experimental_flag filters to experimental build methods before sorting, so a template with an empty "BuildMethod:" (parsed as None) no longer raises TypeError from sorted(). - The --use-container rejection message no longer names "sam build", because ApplicationBuilder also serves sam sync; the docstring now describes when the check runs. - Document why the incremental manifest hash stays keyed by runtime for uv resources: Lambda Builders' uv workflow prefers requirements.txt over pyproject.toml, so hashing requirements.txt matches what is built, and a pyproject-only resource hashes to None and re-downloads every run. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
| "so the Python version to build for can be determined" | ||
| ) | ||
| runtime = compatible_runtimes[0] | ||
| if not runtime.startswith("python"): |
There was a problem hiding this comment.
[INPUT_VALIDATION] runtime.startswith("python") is a prefix test, not a check that the entry is a runtime SAM can build. Anything shaped like python... passes through and reaches Lambda Builders unvalidated.
Concretely, a layer with BuildMethod: python-uv and CompatibleRuntimes: [python3.7] (a plausible leftover in an older template) fails two different ways depending on flags:
- Without
--cached:_build_layerhandspython3.7to_build_function_in_process, and the uv packager derives--python-version 3.7from it. The failure surfaces from inside the builder, not from SAM. - With
--cached:_is_incremental_build_supported("python-uv")is true, soIncrementalBuildStrategyroutes the resolved runtime intoDependencyHashGenerator, whoseget_workflow_config(self._runtime, ...)raises'python3.7' runtime is not supported— a message that names neither the layer norCompatibleRuntimes, which is exactly the class of confusing error this function was added to eliminate.
Note that _build_function screens functions against self._deprecated_runtimes (app_builder.py:745); the layer path has no equivalent, so this is the only place the value gets checked.
Validating against the runtimes the module already knows about keeps the error at the template level and keeps both paths consistent:
runtime = compatible_runtimes[0]
if get_layer_subfolder(runtime) != "python":
raise UnsupportedRuntimeException(
f"BuildMethod '{build_method}' for layers requires a supported Python runtime as the first "
f"CompatibleRuntimes entry, but found '{runtime}'"
)Separately, the hardcoded Python check sits inside a deliberately generic abstraction: DEPENDENCY_MANAGER_BUILD_METHODS is a set, and its comment invites future members. Adding any non-Python dependency-manager build method to that set would make every layer using it fail with "requires a Python runtime". Keying the expected language off the build method's CONFIG (get_workflow_config(None, code_dir, project_dir, build_method).language) rather than a literal "python" would keep the set genuinely extensible.
There was a problem hiding this comment.
Fixed in 3e23c87. resolve_layer_build_runtime now validates the first CompatibleRuntimes entry with get_layer_subfolder and compares its language with the build method's own (get_layer_subfolder(build_method)), instead of a startswith("python") prefix test. python3.7 and nodejs22.x both fail at the template level with a message naming the BuildMethod, CompatibleRuntimes and the offending value, on both the plain and --cached paths. Keying the expected language off the build method keeps DEPENDENCY_MANAGER_BUILD_METHODS extensible without a hardcoded language. I used get_layer_subfolder rather than get_workflow_config(...).language because it needs no code directory and is already the module's source of truth for supported layer runtimes. Unit tests cover python3.7, nodejs22.x, single and multiple runtimes.
…runtimes A prefix test on "python" let unsupported versions such as python3.7 through, to fail later inside Lambda Builders or as a bare "'python3.7' runtime is not supported" from the --cached manifest hash. resolve_layer_build_runtime now checks the first CompatibleRuntimes entry with get_layer_subfolder and compares its language with the build method's own, so the error names the layer's BuildMethod, CompatibleRuntimes and the offending value, and the check stays correct for any future dependency-manager build method. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
| runtime_language: Optional[str] = get_layer_subfolder(runtime) | ||
| except UnsupportedRuntimeException: | ||
| runtime_language = None | ||
| if runtime_language != expected_language: |
There was a problem hiding this comment.
[INPUT_VALIDATION] The new runtime validation is implemented by comparing get_layer_subfolder(runtime) against get_layer_subfolder(build_method), but the same hunk adds "python-uv": "python" to that map. So python-uv now validates as a runtime against itself, and the one value that is definitionally not a runtime passes through:
resolve_layer_build_runtime("python-uv", ["python-uv"])
# expected_language = get_layer_subfolder("python-uv") -> "python"
# runtime_language = get_layer_subfolder("python-uv") -> "python"
# equal -> no raise, returns "python-uv"For a template like
LayerOne:
Type: AWS::Serverless::LayerVersion
Properties:
ContentUri: PyLayer
CompatibleRuntimes:
- python-uv
Metadata:
BuildMethod: python-uvthis reproduces exactly the two failures the PR set out to remove:
- Without
--cached:_build_layersetsbuild_runtime = "python-uv",patch_runtimeleaves it unchanged, and the uv packager derives--python-version -uvagain. - With
--cached:IncrementalBuildStrategyhands"python-uv"toDependencyHashGenerator, whoseget_workflow_config(self._runtime, ...)has nospecified_workflow, so"python-uv"is not inselectors_by_runtimeand it raises'python-uv' runtime is not supported— the bare message the inline comment on line 117 says this check exists to avoid.
Excluding the dependency-manager build methods from the accepted set keeps the check honest and stays correct for future members:
if runtime in DEPENDENCY_MANAGER_BUILD_METHODS or runtime_language != expected_language:
raise UnsupportedRuntimeException(
f"BuildMethod '{build_method}' for layers requires a supported {expected_language} runtime as the first "
f"CompatibleRuntimes entry, but found '{runtime}'"
)There was a problem hiding this comment.
Fixed in d2c9cf7. resolve_layer_build_runtime now rejects any entry in DEPENDENCY_MANAGER_BUILD_METHODS before the language comparison, so CompatibleRuntimes: [python-uv] fails at the template level with the same message as other invalid entries. Added a unit test for ["python-uv", "python3.13"].
| f"CompatibleRuntimes entry, but found '{runtime}'" | ||
| ) | ||
| if len(compatible_runtimes) > 1: | ||
| LOG.warning( |
There was a problem hiding this comment.
[GENERAL] This warning is the user's only signal that SAM silently picked one of several declared runtimes, but it identifies neither the layer nor the chosen-vs-ignored runtimes, and it fires twice per layer under --cached.
resolve_layer_build_runtime is now called from two levels of the same build pipeline for a single layer:
IncrementalBuildStrategy.build_single_layer_definition(build_strategy.py:545)- its delegate
DefaultBuildStrategy→ApplicationBuilder._build_layer(app_builder.py:647)
So sam build --cached on one uv layer with CompatibleRuntimes: [python3.13, python3.12] emits the identical warning twice. With several uv layers in a template the output is a set of repeated, interchangeable lines and the user cannot tell which layer to fix — which defeats the purpose of warning at all.
Since the function does not know the resource, the cheapest fix is to include the ignored runtimes so the message is at least self-identifying, and to emit it from the single call site that knows the layer name (_build_layer has layer_name) rather than from the shared resolver:
if len(compatible_runtimes) > 1:
LOG.warning(
"Layer declares multiple CompatibleRuntimes %s; building with %s for %s. Dependencies compiled for that "
"Python version may not work on %s.",
compatible_runtimes,
runtime,
build_method,
compatible_runtimes[1:],
)There was a problem hiding this comment.
Fixed in d2c9cf7. The warning moved out of the shared resolver into _build_layer, the one call site per layer that knows the layer name, so it fires once under --cached too. It now reads "Layer declares multiple CompatibleRuntimes [...]; building with for python-uv. Dependencies compiled for that Python version may not work on []." A parameterized unit test checks that a single runtime emits no warning and two runtimes emit exactly one naming the layer, the chosen runtime and the ignored ones.
…layer resolve_layer_build_runtime compared the subfolder language of the first CompatibleRuntimes entry with the build method's own, but "python-uv" lives in the same subfolder map, so a template listing python-uv under CompatibleRuntimes passed validation and reached Lambda Builders as the runtime. Dependency-manager build method names are now rejected explicitly. The multiple-CompatibleRuntimes warning moves from the shared resolver, which runs twice per layer under --cached and knows no resource, to _build_layer. It now names the layer, the chosen runtime and the ignored runtimes, and fires once. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
| # Can't prompt for beta confirmation in JSON mode, so skip it - unless the feature is | ||
| # already enabled, where prompt_experimental only updates telemetry and doesn't prompt. | ||
| if self._output is OutputOption.json and not is_experimental_enabled(experimental_flag): | ||
| continue |
There was a problem hiding this comment.
[GENERAL] --output json now silently bypasses the beta gate that text mode enforces.
This PR changes a declined prompt from "discard the answer and build anyway" into a hard abort:
if not prompt_experimental(experimental_flag, WARNING_MESSAGE):
raise UserException(
f'Build method "{build_method}" is a beta feature and was not confirmed. '
'Re-run with "sam build --beta-features" to enable it.'
)But the JSON guard above it still continues:
if self._output is OutputOption.json and not is_experimental_enabled(experimental_flag):
continueBefore this PR both paths ended the same way — the build proceeded — so the guard was consistent. Now the same template diverges purely on the output format:
sam buildon apython-uvresource → prompt; declining fails the build.sam build --output jsonon the same resource → no prompt, no failure, the uv workflow runs unconfirmed.
So --output json becomes a way to use the beta build method without ever consenting to it, which is the outcome the new abort exists to prevent. It also means set_experimental is never called on that path, so get_enabled_experimental_flags() passed down to Lambda Builders in _build_function_in_process omits the uv flag — the build runs in a state the interactive path can never produce.
Since JSON mode structurally cannot obtain consent, the consistent behaviour is to fail with the same message rather than skip the check:
if self._output is OutputOption.json and not is_experimental_enabled(experimental_flag):
raise UserException(
f'Build method "{build_method}" is a beta feature and cannot be confirmed with '
'"--output json". Re-run with "sam build --beta-features" to enable it.'
)This keeps the intent of #9172 (never prompt in JSON mode) while closing the bypass. Note that UserException is not in run()'s except tuple, so _print_build_failure() is skipped — harmless in text mode, but worth confirming do_cli's JSON failure serialization covers it if you take this route.
There was a problem hiding this comment.
Fixed in 59d320d. JSON mode now raises the same UserException with a message pointing at --beta-features instead of skipping the check, so --output json can no longer run the uv workflow unconfirmed. It still never prompts, which keeps the #9172 intent. Verified end to end: sam build --output json on a python-uv layer without the flag prints {"type": "result", "status": "failure", "error": {"type": "UserException", ...}} and exits 1 via the broad catch in command.py; with --beta-features it prints the success document. Also worth noting for the maintainers: text mode without a tty already aborts at click.confirm (click.Abort, exit 1), so JSON mode was the only path that proceeded without consent. This does change #9172's "skip the prompt and proceed" into "require --beta-features (or SAM_CLI_BETA_FEATURES) in JSON mode", which is a deliberate call I am happy to revert if you prefer the previous behaviour.
A declined beta prompt now aborts the build, and click.confirm already aborts without a tty, so skipping the check under --output json left the one path that ran the python-uv workflow without consent and without setting the experimental flag. JSON mode still never prompts; it fails with the same --beta-features remedy, serialized as the usual failure document. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@roger-zhangg the branch is rebased onto
The review bot has no remaining findings on the latest commit (59d320d). The |
Summary
python-uvas a validBuildMethodforAWS::Serverless::LayerVersion/AWS::Lambda::LayerVersion. Previously layers hitUnsupportedRuntimeExceptionatget_layer_subfolder.CompatibleRuntimesentry (resolve_layer_build_runtime).python-uvis a workflow name, not a runtime, and the uv packager derives--python-versionfrom it, so passing it through produced--python-version -uv. A uv layer withoutCompatibleRuntimesnow fails with a clear message.--cached:IncrementalBuildStrategynow hands the same resolved runtime toDependencyHashGeneratorinstead of the rawBuildMethod, so cached uv layer builds no longer fail with'python-uv' runtime is not supported. The incremental uv path itself was fixed upstream in aws-lambda-builders 1.67.0 (fix(python-uv): cached build copies dependencies to the build dir aws-lambda-builders#870), whichdevelopalready pins.--use-container: rejected up front for uv builds (functions and layers) with an actionable message. The SAM build images do not ship uv yet (the uv container cases intest_build_cmd_python.pyare commented out for that reason), and the previous behaviour was aTypeErrorinsideLambdaBuildContainerbecausePYTHON_UV_CONFIGhas no manifest to mount._check_build_method_experimental_flagnow covers layers, prompts once per distinct build method (in sorted order), preserves the--output jsonguard from feat: output json integration #9172, and aborts the build when the prompt is declined instead of discarding the answer.Test plan
tests/unit/lib/build_module/test_workflow_config.py:get_layer_subfolderandresolve_layer_build_runtime(pass-through, uv resolution, missingCompatibleRuntimes)tests/unit/lib/build_module/test_app_builder.py:_build_layerasserts the runtime handed to the builder for a uv layer, raises withoutCompatibleRuntimes, and rejects container builds for uv layers and functionstests/unit/lib/build_module/test_build_strategy.py: incremental layer manifest hash uses the resolved runtimetests/unit/commands/buildcmd/test_build_context.py: layer prompt, dedup, JSON guard, abort on declinetests/integration/buildcmd/test_build_cmd.py::TestBuildCommand_LayerBuilds: real uv layer builds forLayerOne/LambdaLayerOne,--cachedcold + warm, missingCompatibleRuntimeserror,--use-containerrejection. All 5 pass locally (macOS, uv 0.12.10, Docker up).ruff check samcli,black --check,mypyclean on touched files🤖 Generated with Claude Code