Skip to content

Commit adeb8d8

Browse files
committed
fix(): address comments
1 parent 2e790a0 commit adeb8d8

File tree

8 files changed

+93
-76
lines changed

8 files changed

+93
-76
lines changed

.github/workflows/build.yml

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ jobs:
3030
- java-maven-integration
3131
- java-gradle-integration
3232
- custom-make-integration
33-
- python-integration
33+
- python-pip-integration
34+
- python-uv-integration
3435
- ruby-integration
3536
- dotnet-integration
3637
- rust-cargo-lambda-integration
@@ -44,7 +45,8 @@ jobs:
4445
needs.java-maven-integration.result != 'success' ||
4546
needs.java-gradle-integration.result != 'success' ||
4647
needs.custom-make-integration.result != 'success' ||
47-
needs.python-integration.result != 'success' ||
48+
needs.python-pip-integration.result != 'success' ||
49+
needs.python-uv-integration.result != 'success' ||
4850
needs.ruby-integration.result != 'success' ||
4951
needs.dotnet-integration.result != 'success' ||
5052
needs.rust-cargo-lambda-integration.result != 'success'
@@ -253,8 +255,37 @@ jobs:
253255
- run: make init
254256
- run: pytest -vv tests/integration/workflows/custom_make
255257

256-
python-integration:
257-
name: ${{ matrix.os }} / ${{ matrix.python }} / python (pip + uv)
258+
python-pip-integration:
259+
name: ${{ matrix.os }} / ${{ matrix.python }} / python-pip
260+
if: github.repository_owner == 'aws'
261+
runs-on: ${{ matrix.os }}
262+
strategy:
263+
fail-fast: false
264+
matrix:
265+
os:
266+
- ubuntu-latest
267+
- windows-latest
268+
python:
269+
- "3.8"
270+
- "3.9"
271+
- "3.10"
272+
- "3.11"
273+
- "3.12"
274+
- "3.13"
275+
steps:
276+
- uses: actions/checkout@v4
277+
- uses: actions/setup-python@v5
278+
with:
279+
python-version: ${{ matrix.python }}
280+
- run: |
281+
python -m pip install --upgrade pip
282+
pip install --upgrade setuptools
283+
if: ${{ matrix.os }} == 'ubuntu-latest' && ${{ matrix.python }} == '3.12'
284+
- run: make init
285+
- run: pytest -vv tests/integration/workflows/python_pip
286+
287+
python-uv-integration:
288+
name: ${{ matrix.os }} / ${{ matrix.python }} / python-uv
258289
if: github.repository_owner == 'aws'
259290
runs-on: ${{ matrix.os }}
260291
strategy:
@@ -285,8 +316,6 @@ jobs:
285316
with:
286317
enable-cache: true
287318
- run: make init
288-
# Test both python_pip and python_uv workflows
289-
- run: pytest -vv tests/integration/workflows/python_pip
290319
- run: pytest -vv tests/integration/workflows/python_uv
291320

292321
ruby-integration:

aws_lambda_builders/workflows/python_uv/DESIGN.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,9 +278,8 @@ Note: `requirements.in` (pip-tools format) is not supported to keep the implemen
278278
- Fallback: Attempt to install UV using pip if not found (optional behavior)
279279

280280
#### Error Handling Strategy
281-
- **MissingUvError**: UV binary not found on PATH
281+
- **MissingUvError**: UV binary not found on PATH (includes path information)
282282
- **UvInstallationError**: UV installation/setup failures
283-
- **UvResolutionError**: Dependency resolution failures
284283
- **UvBuildError**: Package build failures
285284
- **LockFileError**: Lock file parsing or validation errors
286285

aws_lambda_builders/workflows/python_uv/actions.py

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -80,24 +80,3 @@ def execute(self) -> None:
8080
except Exception as ex:
8181
LOG.error("Unexpected error during UV build: %s", str(ex))
8282
raise ActionFailedError(f"UV build failed: {str(ex)}")
83-
84-
85-
class CopyDependenciesAction(BaseAction):
86-
"""
87-
Custom action for copying dependencies from dependencies_dir to artifacts_dir.
88-
Provides clear action name to distinguish from source code copying.
89-
"""
90-
91-
NAME = "CopyDependencies"
92-
DESCRIPTION = "Copying dependencies from dependencies directory to artifacts directory"
93-
PURPOSE = Purpose.COPY_SOURCE
94-
95-
def __init__(self, dependencies_dir, artifacts_dir):
96-
from aws_lambda_builders.actions import copytree
97-
98-
self.dependencies_dir = dependencies_dir
99-
self.artifacts_dir = artifacts_dir
100-
self._copytree = copytree
101-
102-
def execute(self):
103-
self._copytree(self.dependencies_dir, self.artifacts_dir, maintain_symlinks=False)

aws_lambda_builders/workflows/python_uv/exceptions.py

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
class MissingUvError(LambdaBuilderError):
99
"""Exception raised when UV executable is not found."""
1010

11-
MESSAGE = "uv executable not found in your environment. Please install uv: https://docs.astral.sh/uv/getting-started/installation/"
11+
MESSAGE = "uv executable not found at {uv_path}. Please install uv: https://docs.astral.sh/uv/getting-started/installation/"
1212

1313

1414
class UvInstallationError(LambdaBuilderError):
@@ -17,12 +17,6 @@ class UvInstallationError(LambdaBuilderError):
1717
MESSAGE = "Failed to install dependencies using uv: {reason}"
1818

1919

20-
class UvResolutionError(LambdaBuilderError):
21-
"""Exception raised when UV dependency resolution fails."""
22-
23-
MESSAGE = "UV dependency resolution failed: {reason}"
24-
25-
2620
class UvBuildError(LambdaBuilderError):
2721
"""Exception raised when UV package build fails."""
2822

@@ -33,9 +27,3 @@ class LockFileError(LambdaBuilderError):
3327
"""Exception raised when lock file operations fail."""
3428

3529
MESSAGE = "Lock file operation failed: {reason}"
36-
37-
38-
class ManifestNotFoundError(LambdaBuilderError):
39-
"""Exception raised when no supported manifest file is found."""
40-
41-
MESSAGE = "No supported dependency manifest found. Expected one of: {supported_manifests}"

aws_lambda_builders/workflows/python_uv/packager.py

Lines changed: 44 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def _find_uv_executable(self) -> str:
2727
"""Find UV executable in PATH."""
2828
uv_path = self._osutils.which("uv")
2929
if not uv_path:
30-
raise MissingUvError()
30+
raise MissingUvError(uv_path="not found in PATH")
3131
return uv_path
3232

3333
@property
@@ -76,14 +76,19 @@ def uv_version(self) -> Optional[str]:
7676
"""Get UV version."""
7777
return get_uv_version(self._uv.uv_executable, self._osutils)
7878

79+
def _ensure_cache_dir(self, config: UvConfig, scratch_dir: str) -> None:
80+
"""Ensure UV cache directory is configured."""
81+
if not config.cache_dir:
82+
config.cache_dir = os.path.join(scratch_dir, "uv-cache")
83+
if not os.path.exists(config.cache_dir):
84+
self._osutils.makedirs(config.cache_dir)
85+
7986
def sync_dependencies(
8087
self,
8188
target_dir: str,
8289
scratch_dir: str,
8390
config: Optional[UvConfig] = None,
8491
python_version: Optional[str] = None,
85-
platform: Optional[str] = None,
86-
architecture: Optional[str] = None,
8792
manifest_path: Optional[str] = None,
8893
project_dir: Optional[str] = None,
8994
) -> None:
@@ -95,8 +100,6 @@ def sync_dependencies(
95100
scratch_dir: Scratch directory for temporary operations
96101
config: UV configuration options
97102
python_version: Target Python version (e.g., "3.9")
98-
platform: Target platform (e.g., "linux")
99-
architecture: Target architecture (e.g., "x86_64")
100103
manifest_path: Path to dependency manifest file (for backwards compatibility)
101104
project_dir: Project directory containing pyproject.toml and uv.lock
102105
"""
@@ -113,11 +116,7 @@ def sync_dependencies(
113116
raise ValueError("Either project_dir or manifest_path must be provided")
114117

115118
# Ensure UV cache is configured to use scratch directory
116-
if not config.cache_dir:
117-
config.cache_dir = os.path.join(scratch_dir, "uv-cache")
118-
# Use exist_ok equivalent for osutils
119-
if not os.path.exists(config.cache_dir):
120-
self._osutils.makedirs(config.cache_dir)
119+
self._ensure_cache_dir(config, scratch_dir)
121120

122121
args = ["sync"]
123122

@@ -136,6 +135,8 @@ def sync_dependencies(
136135

137136
if rc != 0:
138137
raise UvInstallationError(reason=f"UV sync failed: {stderr}")
138+
139+
LOG.debug("UV sync completed successfully: %s", stdout)
139140

140141
# Copy dependencies from virtual environment to target directory
141142
# uv sync creates a .venv directory in the project directory
@@ -180,11 +181,7 @@ def install_requirements(
180181
config = UvConfig()
181182

182183
# Ensure UV cache is configured to use scratch directory
183-
if not config.cache_dir:
184-
config.cache_dir = os.path.join(scratch_dir, "uv-cache")
185-
# Use exist_ok equivalent for osutils
186-
if not os.path.exists(config.cache_dir):
187-
self._osutils.makedirs(config.cache_dir)
184+
self._ensure_cache_dir(config, scratch_dir)
188185

189186
args = ["pip", "install"]
190187

@@ -218,6 +215,8 @@ def install_requirements(
218215

219216
if rc != 0:
220217
raise UvInstallationError(reason=f"UV pip install failed: {stderr}")
218+
219+
LOG.debug("UV pip install completed successfully: %s", stdout)
221220

222221

223222
class PythonUvDependencyBuilder:
@@ -263,12 +262,9 @@ def build_dependencies(
263262

264263
# Configure UV to use scratch directory for cache if not already set
265264
if not config.cache_dir:
266-
uv_cache_dir = os.path.join(scratch_dir_path, "uv-cache")
267-
# Use exist_ok equivalent for osutils
268-
if not os.path.exists(uv_cache_dir):
269-
self._osutils.makedirs(uv_cache_dir)
270-
config.cache_dir = uv_cache_dir
271-
LOG.debug("Configured UV cache directory: %s", uv_cache_dir)
265+
config.cache_dir = os.path.join(scratch_dir_path, "uv-cache")
266+
if not os.path.exists(config.cache_dir):
267+
self._osutils.makedirs(config.cache_dir)
272268

273269
# Determine Python version from runtime
274270
python_version = self._extract_python_version(self.runtime)
@@ -277,19 +273,31 @@ def build_dependencies(
277273
manifest_name = os.path.basename(manifest_path)
278274

279275
try:
280-
# Get the appropriate handler for this manifest
276+
# Get the appropriate build handler based on manifest type
277+
# This dispatch system allows different build strategies:
278+
# - pyproject.toml: Uses uv sync (with lock) or uv lock + export (without lock)
279+
# - requirements.txt: Uses uv pip install with platform targeting
281280
handler = self._get_manifest_handler(manifest_name)
282281

283-
# Execute the handler
282+
# Execute the selected build strategy
283+
# Dependencies are built by the handler methods which call UvRunner:
284+
# - UvRunner.sync_dependencies(): For pyproject.toml with uv.lock
285+
# - UvRunner.pip_install(): For requirements.txt and pyproject.toml without lock
286+
# Both methods install packages and copy site-packages to artifacts_dir_path
284287
handler(manifest_path, artifacts_dir_path, scratch_dir_path, python_version, architecture, config)
285288

286289
except Exception as e:
287290
LOG.error("Failed to build dependencies: %s", str(e))
288291
raise
289292

290293
def _get_manifest_handler(self, manifest_name: str):
291-
"""Get the appropriate handler function for a manifest file."""
292-
# Exact match handlers for ACTUAL manifests
294+
"""Get the appropriate handler function for a manifest file.
295+
296+
Uses a dictionary-based dispatch pattern for extensibility.
297+
This allows easy addition of new manifest types in the future
298+
without modifying the core dispatch logic.
299+
"""
300+
# Exact match handlers for specific manifest files
293301
exact_handlers = {
294302
"pyproject.toml": self._handle_pyproject_build,
295303
}
@@ -417,7 +425,16 @@ def _build_from_pyproject(
417425
def _export_pyproject_to_requirements(
418426
self, pyproject_path: str, scratch_dir: str, python_version: str
419427
) -> Optional[str]:
420-
"""Use UV's native lock and export to convert pyproject.toml to requirements.txt."""
428+
"""Use UV's native lock and export to convert pyproject.toml to requirements.txt.
429+
430+
This conversion is necessary when pyproject.toml exists without a uv.lock file.
431+
UV's pip install command provides better platform targeting capabilities
432+
(--python-platform) compared to uv sync, which is essential for Lambda's
433+
cross-platform builds (x86_64/ARM64). The workflow is:
434+
1. uv lock: Generate lock file from pyproject.toml
435+
2. uv export: Convert lock file to requirements.txt format
436+
3. Use requirements.txt with uv pip install for platform-specific builds
437+
"""
421438
project_dir = os.path.dirname(pyproject_path)
422439

423440
try:
@@ -492,7 +509,7 @@ def _build_from_requirements(
492509
except Exception as e:
493510
raise UvBuildError(reason=f"Failed to build from requirements: {str(e)}")
494511

495-
def _extract_python_version(self, runtime: str) -> str:
512+
def _extract_python_version(self, runtime: Optional[str]) -> str:
496513
"""Extract Python version from runtime string."""
497514
if not runtime:
498515
raise UvBuildError(reason="Runtime is required but was not provided")

aws_lambda_builders/workflows/python_uv/workflow.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44

55
import logging
66

7-
from aws_lambda_builders.actions import CleanUpAction, CopySourceAction
7+
from aws_lambda_builders.actions import CleanUpAction, CopySourceAction, CopyDependenciesAction
88
from aws_lambda_builders.path_resolver import PathResolver
99
from aws_lambda_builders.workflow import BaseWorkflow, BuildDirectory, BuildInSourceSupport, Capability
1010

11-
from .actions import CopyDependenciesAction, PythonUvBuildAction
11+
from .actions import PythonUvBuildAction
1212
from .utils import OSUtils, detect_uv_manifest
1313

1414
LOG = logging.getLogger(__name__)
@@ -144,7 +144,14 @@ def _setup_build_actions(self, source_dir, artifacts_dir, scratch_dir, manifest_
144144

145145
# Advanced case: Copy dependencies from dependencies_dir to artifacts_dir if configured
146146
if self.dependencies_dir and self.combine_dependencies:
147-
self.actions.append(CopyDependenciesAction(self.dependencies_dir, artifacts_dir))
147+
self.actions.append(
148+
CopyDependenciesAction(
149+
source_dir=source_dir,
150+
artifact_dir=artifacts_dir,
151+
destination_dir=self.dependencies_dir,
152+
maintain_symlinks=False,
153+
)
154+
)
148155

149156
# Always copy source code (final step)
150157
self.actions.append(CopySourceAction(source_dir, artifacts_dir, excludes=self.EXCLUDED_FILES))

tests/unit/workflows/python_uv/test_packager.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,6 @@ def test_sync_dependencies_success(self):
6565
target_dir="/target",
6666
scratch_dir="/scratch",
6767
python_version="3.9",
68-
platform="linux",
69-
architecture=X86_64,
7068
)
7169

7270
# Verify UV sync command was called with correct arguments

tests/unit/workflows/python_uv/test_workflow.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33

44
from parameterized import parameterized_class
55

6-
from aws_lambda_builders.actions import CopySourceAction, CleanUpAction, LinkSourceAction
6+
from aws_lambda_builders.actions import CopySourceAction, CleanUpAction, LinkSourceAction, CopyDependenciesAction
77
from aws_lambda_builders.path_resolver import PathResolver
88
from aws_lambda_builders.workflows.python_uv.utils import OSUtils, EXPERIMENTAL_FLAG_BUILD_PERFORMANCE
99
from aws_lambda_builders.workflows.python_uv.workflow import PythonUvWorkflow
10-
from aws_lambda_builders.workflows.python_uv.actions import PythonUvBuildAction, CopyDependenciesAction
10+
from aws_lambda_builders.workflows.python_uv.actions import PythonUvBuildAction
1111

1212

1313
@parameterized_class(

0 commit comments

Comments
 (0)