diff --git a/aws_lambda_builders/workflows/python_uv/DESIGN.md b/aws_lambda_builders/workflows/python_uv/DESIGN.md index 4c5de87d9..61a27c092 100644 --- a/aws_lambda_builders/workflows/python_uv/DESIGN.md +++ b/aws_lambda_builders/workflows/python_uv/DESIGN.md @@ -201,7 +201,7 @@ The workflow supports various configuration options through the config parameter config = { "index_url": "https://pypi.org/simple/", # Custom package index "extra_index_urls": [], # Additional package indexes - "cache_dir": "/tmp/uv-cache", # Custom cache directory + "cache_dir": None, # Custom cache directory; UV's own default when unset "no_cache": False, # Disable caching "prerelease": "disallow", # Handle pre-release versions "resolution": "highest", # Resolution strategy @@ -211,6 +211,19 @@ config = { } ``` +### Caching + +The workflow does not pass `--cache-dir` unless a caller supplies one, so UV uses its own +default cache location (`~/.cache/uv` on Linux, overridable with `UV_CACHE_DIR`). This mirrors +the `python_pip` workflow, which likewise leaves `PIP_CACHE_DIR` alone and inherits pip's +user-level cache. + +Keeping the cache outside the build directory is what makes it useful: it survives a single +build, so UV reuses previously downloaded dependencies across functions within one `sam build` +and across successive builds. Pointing UV at the build's scratch directory instead would +discard the cache every time, because that directory is deleted when the build finishes — every +function would then re-download its entire dependency set on every build. + ### Compatibility with Existing Workflows The UV workflow is designed to be a drop-in replacement for the pip workflow: diff --git a/aws_lambda_builders/workflows/python_uv/packager.py b/aws_lambda_builders/workflows/python_uv/packager.py index 291c9c191..b6c88a9d7 100644 --- a/aws_lambda_builders/workflows/python_uv/packager.py +++ b/aws_lambda_builders/workflows/python_uv/packager.py @@ -95,18 +95,10 @@ def uv_version(self) -> Optional[str]: """Get UV version.""" return self._uv.get_uv_version() - def _ensure_cache_dir(self, config: UvConfig, scratch_dir: str) -> None: - """Ensure UV cache directory is configured.""" - if not config.cache_dir: - config.cache_dir = os.path.join(scratch_dir, "uv-cache") - if not os.path.exists(config.cache_dir): - self._osutils.makedirs(config.cache_dir) - def install_requirements( self, requirements_path: str, target_dir: str, - scratch_dir: str, config: Optional[UvConfig] = None, python_version: Optional[str] = None, platform: Optional[str] = None, @@ -116,10 +108,15 @@ def install_requirements( """ Install requirements using UV pip interface. + No ``--cache-dir`` is passed unless the caller supplies one, so UV uses its own + default cache location. That cache outlives a single build, letting UV reuse + previously downloaded dependencies across functions and across builds. Pointing UV + at a per-build directory instead would discard the cache every time, since the + build's scratch directory is deleted when the build finishes. + Args: requirements_path: Path to requirements.txt file target_dir: Directory to install dependencies - scratch_dir: Scratch directory for temporary operations config: UV configuration options python_version: Target Python version platform: Target platform @@ -128,9 +125,6 @@ def install_requirements( if config is None: config = UvConfig() - # Ensure UV cache is configured to use scratch directory - self._ensure_cache_dir(config, scratch_dir) - args = ["pip", "install"] # Add requirements file @@ -348,7 +342,6 @@ def _build_from_lock_file( self._uv_runner.install_requirements( requirements_path=temp_requirements, target_dir=target_dir, - scratch_dir=scratch_dir, config=config, python_version=python_version, platform="linux", @@ -400,14 +393,17 @@ def _build_from_requirements( architecture: str, config: UvConfig, ) -> None: - """Build dependencies from requirements.txt file.""" + """Build dependencies from requirements.txt file. + + ``scratch_dir`` is unused here; it is part of the signature shared by every manifest + handler dispatched from :meth:`build_dependencies`. + """ LOG.info("Building from requirements file") try: self._uv_runner.install_requirements( requirements_path=requirements_path, target_dir=target_dir, - scratch_dir=scratch_dir, config=config, python_version=python_version, platform="linux", diff --git a/tests/unit/workflows/python_uv/test_packager.py b/tests/unit/workflows/python_uv/test_packager.py index 5f9b235e6..1c199ae24 100644 --- a/tests/unit/workflows/python_uv/test_packager.py +++ b/tests/unit/workflows/python_uv/test_packager.py @@ -112,7 +112,6 @@ def test_install_requirements_success(self): self.uv_runner.install_requirements( requirements_path="/path/to/requirements.txt", target_dir="/target", - scratch_dir="/scratch", python_version="3.9", platform="linux", architecture=X86_64, @@ -133,7 +132,6 @@ def test_install_requirements_resolves_relative_target_to_absolute(self): self.uv_runner.install_requirements( requirements_path="/path/to/requirements.txt", target_dir=os.path.join(".aws-sam", "deps", "abc-123"), - scratch_dir="/scratch", ) args_called = self.mock_subprocess_uv.run_uv_command.call_args[0][0] @@ -141,13 +139,50 @@ def test_install_requirements_resolves_relative_target_to_absolute(self): self.assertTrue(os.path.isabs(target_value), f"--target should be absolute, got: {target_value}") self.assertEqual(target_value, os.path.abspath(os.path.join(".aws-sam", "deps", "abc-123"))) + def test_install_requirements_does_not_pass_cache_dir_by_default(self): + # No --cache-dir means UV uses its own persistent cache, so dependencies downloaded for + # one function are reused by the next function and by later builds. Passing a per-build + # directory here would throw the cache away every time. + self.mock_subprocess_uv.run_uv_command.return_value = (0, "success", "") + + self.uv_runner.install_requirements( + requirements_path="/path/to/requirements.txt", + target_dir="/target", + ) + + args_called = self.mock_subprocess_uv.run_uv_command.call_args[0][0] + self.assertNotIn("--cache-dir", args_called) + + def test_install_requirements_does_not_create_cache_directory(self): + # The previous implementation created a cache directory on disk under the scratch dir. + self.mock_subprocess_uv.run_uv_command.return_value = (0, "success", "") + + self.uv_runner.install_requirements( + requirements_path="/path/to/requirements.txt", + target_dir="/target", + ) + + self.mock_osutils.makedirs.assert_not_called() + + def test_install_requirements_honors_caller_supplied_cache_dir(self): + # An explicit cache_dir is still forwarded, so a caller can opt into a specific location. + self.mock_subprocess_uv.run_uv_command.return_value = (0, "success", "") + + self.uv_runner.install_requirements( + requirements_path="/path/to/requirements.txt", + target_dir="/target", + config=UvConfig(cache_dir="/custom/cache"), + ) + + args_called = self.mock_subprocess_uv.run_uv_command.call_args[0][0] + self.assertIn("--cache-dir", args_called) + self.assertEqual(args_called[args_called.index("--cache-dir") + 1], "/custom/cache") + def test_install_requirements_failure(self): self.mock_subprocess_uv.run_uv_command.return_value = (1, "", "error message") with self.assertRaises(UvInstallationError): - self.uv_runner.install_requirements( - requirements_path="/path/to/requirements.txt", target_dir="/target", scratch_dir="/scratch" - ) + self.uv_runner.install_requirements(requirements_path="/path/to/requirements.txt", target_dir="/target") class TestPythonUvDependencyBuilder(TestCase): @@ -288,8 +323,13 @@ def test_build_dependencies_pyproject_without_uv_lock(self): # Verify it checked for uv.lock in the right location mock_exists.assert_called_with(os.path.join("path", "to", "uv.lock")) - def test_build_dependencies_passes_scratch_dir(self): - """Test that build_dependencies passes scratch_dir to UvRunner for cache configuration.""" + def test_build_dependencies_leaves_cache_dir_unset(self): + """A build must not derive a cache directory from the ephemeral scratch directory. + + The scratch directory is deleted when the build finishes, so caching there would make + every function re-download its dependencies on every build. Leaving cache_dir unset + lets UV use its own persistent cache. + """ with patch("os.path.basename", return_value="requirements.txt"): self.builder.build_dependencies( artifacts_dir_path="/artifacts", @@ -298,10 +338,8 @@ def test_build_dependencies_passes_scratch_dir(self): architecture=X86_64, ) - # Verify that install_requirements was called with scratch_dir - # UvRunner._ensure_cache_dir() will use this to configure the cache - call_args = self.mock_uv_runner.install_requirements.call_args - self.assertEqual(call_args[1]["scratch_dir"], "/scratch") + passed_config = self.mock_uv_runner.install_requirements.call_args[1]["config"] + self.assertIsNone(passed_config.cache_dir) def test_build_dependencies_respects_existing_cache_dir(self): """Test that existing cache_dir in config is respected."""