-
Notifications
You must be signed in to change notification settings - Fork 164
fix(python-uv): preserve compiled bytecode after packaging #926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
c2d25c3
4300971
6848bc2
514513d
0facd03
34b4881
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,17 @@ def get_uv_version(self) -> Optional[str]: | |
| pass | ||
| return None | ||
|
|
||
| def find_python(self, python_version: str) -> Optional[str]: | ||
| """Find an already-installed interpreter matching the requested Python version.""" | ||
| rc, stdout, stderr = self.run_uv_command( | ||
| ["python", "find", "--no-project", "--no-python-downloads", python_version] | ||
| ) | ||
| if rc == 0 and stdout: | ||
| return stdout.strip() | ||
| diagnostic = (stderr or "").strip() or (stdout or "").strip() or "no diagnostic output" | ||
| LOG.warning("Could not locate target Python %s via uv (exit code %d): %s", python_version, rc, diagnostic) | ||
| return None | ||
|
|
||
| def run_uv_command(self, args: List[str], cwd: Optional[str] = None, env: Optional[Dict[str, str]] = None) -> tuple: | ||
| """ | ||
| Execute UV command with given arguments. | ||
|
|
@@ -133,15 +144,33 @@ def install_requirements( | |
| # Resolve --target to an absolute path: UV runs with cwd set to the project directory, so a | ||
| # relative target (e.g. the incremental-build dependencies dir) would otherwise be created | ||
| # under the source directory instead of the build root. | ||
| args.extend(["--target", os.path.abspath(target_dir)]) | ||
| target_dir = os.path.abspath(target_dir) | ||
| args.extend(["--target", target_dir]) | ||
|
|
||
| # Add configuration arguments | ||
| target_python = None | ||
| if config.compile_bytecode: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [GENERAL] Bytecode compilation is effectively enabled for every build, and there is no way to turn it off.
Before this PR nothing was compiled, because The impact is concrete: If opt-in was the intent, flip the default: compile_bytecode: bool = False,If default-on is intentional, please say so explicitly, since it changes artifact size and build time for all existing users of this workflow. |
||
| if python_version: | ||
| target_python = self._uv.find_python(python_version) | ||
| else: | ||
| LOG.warning("Target Python version is unavailable; skipping bytecode compilation") | ||
|
|
||
| # Add configuration arguments that are independent of interpreter selection. | ||
| args.extend(config.to_uv_args()) | ||
|
|
||
| # Add platform-specific arguments | ||
| if python_version: | ||
| args.extend(["--python-version", python_version]) | ||
|
|
||
| # Use the exact interpreter found by UV instead of repeating a version request that | ||
| # could resolve differently when the install command runs. | ||
| if target_python: | ||
| args.extend(["--python", target_python]) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [GENERAL] The
command = [python_executable, "-m", "compileall", "-f", "-q", "--invalidation-mode", "unchecked-hash", target_dir]The resulting Dropping the pin keeps the install behavior identical to today and confines the new code path to the compilation step: if python_version:
args.extend(["--python-version", python_version])with |
||
|
|
||
| # UV currently produces timestamp-based bytecode. SAM packages files with normalized | ||
| # timestamps, so that bytecode would be rejected by the Lambda runtime after deployment. | ||
| # Keep UV compilation disabled and generate unchecked-hash bytecode below instead. | ||
| args.append("--no-compile-bytecode") | ||
|
|
||
| if platform and architecture: | ||
| # UV pip install uses --python-platform format | ||
| # Map Lambda architectures to UV platform strings | ||
|
|
@@ -162,6 +191,34 @@ def install_requirements( | |
|
|
||
| LOG.debug("UV pip install completed successfully: %s", stdout) | ||
|
|
||
| if target_python: | ||
| self._compile_bytecode(target_python, target_dir) | ||
|
|
||
| def _compile_bytecode(self, python_executable: str, target_dir: str) -> None: | ||
| """Compile reusable bytecode with the interpreter selected for the target runtime.""" | ||
| command = [ | ||
| python_executable, | ||
| "-m", | ||
| "compileall", | ||
| "-f", | ||
| "-q", | ||
| "--invalidation-mode", | ||
| "unchecked-hash", | ||
| target_dir, | ||
| ] | ||
| LOG.debug("Compiling unchecked-hash bytecode: %s", " ".join(command)) | ||
| rc, stdout, stderr = self._osutils.run_subprocess(command) | ||
| if rc != 0: | ||
| diagnostic = stderr.strip() or stdout.strip() or "no diagnostic output" | ||
| LOG.warning( | ||
| "Could not compile unchecked-hash bytecode with target Python (exit code %d): %s", | ||
| rc, | ||
| diagnostic, | ||
| ) | ||
| return | ||
|
|
||
| LOG.debug("Python bytecode compilation completed successfully: %s", stdout) | ||
|
|
||
|
|
||
| class PythonUvDependencyBuilder: | ||
| """High-level dependency builder that orchestrates UV operations.""" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[GENERAL] A missing target interpreter is the normal case for cross-version builds, so
LOG.warninghere will surface a scary message on healthy builds.Building a
python3.9function on a host that only has 3.12 installed — or any build in an image whose interpreter differs from the runtime — hits this path, and--no-python-downloadsguarantees uv will not fill the gap. Since bytecode compilation is a best-effort optimization whose absence has no effect on the produced artifact, this belongs atLOG.debug(orLOG.info), otherwise users see a warning insam buildoutput for a build that succeeded exactly as intended. Note this is amplified by the default in comment 1: with compilation on by default, most cross-version builds will emit it.The same applies to the "Target Python version is unavailable" warning at line 155 —
_extract_python_version()raises when there is no runtime, so a falsypython_versionreachinginstall_requirements()only happens for direct library callers.