Warn about `files()` in PEXes in one place #19027

huonw · 2023-05-17T22:35:53Z

There's several places that check whether a PEX transitively depends on any files() targets, because those are (currently) not included in the PEX. This moves the warning into the "pex from targets" builder, to not have to reimplement it everywhere.

This is follow up to #19022, and restores the warning for layout="zip", since I didn't implement it there.

This fixes #18879 by allowing the `python_awslambda` and `python_google_cloud_function` FaaS artefacts to be generated in "simple" format, using the `pex3 venv create --layout=flat-zipped` functionality recently added in PEX 2.1.135 (https://github.com/pantsbuild/pex/releases/tag/v2.1.135). This format is just: put everything at the top-level, e.g. the zip contains `cowsay/__init__.py` etc., rather than `.deps/cowsay-....whl` (plus the dynamic PEX initialisation). This shifts the dynamic dependency computation/extraction/layout from run-time to build-time, relying on the FaaS environment to be generally consistent. It shouldn't change what actually happens after initialisation. This can: - reduce cold-starts noticeably: for instance, some of our lambdas spend 1s doing PEX/Lambdex start up. - reduce package size somewhat (the PEX `.bootstrap/` folder seems to be about 2MB uncompressed, ~1MB compressed). - increase build times. For instance, for one Python 3.9 Lambda in our codebase: | metric | before | after | |-------------------------------|----------|------------------| | init time on cold start | 2.3-2.5s | 1.3-1.4s (-1s) | | compressed size | 24.6MB | 23.8MB (-0.8MB) | | uncompressed size | 117.8MB | 115.8MB (-2.0MB) | | PEX-construction build time | ~5s | ~5s | | PEX-postprocessing build time | 0.14s | 4.8s | (The PEX-postprocessing time metric is specifically the time to run the `Setting up handler` (lambdex) or `Build python_awslambda` (`pex3 venv create`) process, computed by running `pants --keep-sandboxes=always package ...` for each layout, and then `hyperfine -r3 -w1 path/to/first/__run.sh path/to/second/__run.sh`. This _doesn't_ include the time to construct the input PEX, which is the same for both.) This functionality is driven by adding a new `layout` field. It defaults to `lambdex` (retaining the current code paths), but also supports `zip`, which keys into the functionality above. I've tried to keep the non-lambdex implementation generally separate to the lambdex one, rather than reusing all of the code that happens to be common currently, because it'd make sense to deprecate/remove the lambdex functionality and thus I feel it's best for this new functionality to be mostly a fresh start. This PR's commits can be reviewed independently. It comes in three phases: 1. Add the `pex_venv.py` util rules for running `pex3 venv create ...`. Currently this only supports a limited subset of the functionality there, but can presumably be expanded freely as required. (First commit) 2. Do some minor refactoring. (Commits labelled "refactor: ...") 3. Draw the rest of the owl. (The others.) I _think_ this is an acceptable MVP for this functionality, but there's various bits of follow-up: - deprecate `layout="lambdex"` (in favour of `layout="zip"` and/or normal `pex_binary`) (#19032) - add a warning about `files` being loaded into these packages, which has been temporarily lost (#19027) - adjust documentation - other improvements like #18195 and #18880 - improve performance, e.g. potentially `pex3 venv create ...` could use the lock file and sources to directly compute the appropriate files, without having to materialise a normal pex first

src/python/pants/backend/python/util_rules/pex_from_targets.py

This fixes #18879 by allowing the `python_awslambda` and `python_google_cloud_function` FaaS artefacts to be generated in "simple" format, using the `pex3 venv create --layout=flat-zipped` functionality recently added in PEX 2.1.135 (https://github.com/pantsbuild/pex/releases/tag/v2.1.135). This format is just: put everything at the top-level. For instance, the zip contains `cowsay/__init__.py` etc., rather than `.deps/cowsay-....whl`. This avoids the need to do the dynamic PEX initialisation/venv creation. This shifts the dynamic dependency computation/extraction/layout from run-time to build-time, relying on the FaaS environment to be generally consistent. It shouldn't change what actually happens after initialisation. This can: - reduce cold-starts noticeably: for instance, some of our lambdas spend 1s doing PEX/Lambdex start up. - reduce package size somewhat (the PEX `.bootstrap/` folder seems to be about 2MB uncompressed, ~1MB compressed). - increase build times. For instance, for one Python 3.9 Lambda in our codebase: | metric | before | after | |---|---|---| | init time on cold start | 2.3-2.5s | 1.3-1.4s (-1s) | | compressed size | 24.6MB | 23.8MB (-0.8MB) | | uncompressed size | 117.8MB | 115.8MB (-2.0MB) | | PEX-construction build time | ~5s | ~5s | | PEX-postprocessing build time | 0.14s | 4.8s | (The PEX-postprocessing time metric is specifically the time to run the `Setting up handler` (lambdex) or `Build python_awslambda` (`pex3 venv create`) process, computed by running `pants --keep-sandboxes=always package ...` for each layout, and then `hyperfine -r3 -w1 path/to/first/__run.sh path/to/second/__run.sh`. This _doesn't_ include the time to construct the input PEX, which is the same for both.) --- This functionality is driven by adding a new option to the `[lambdex].layout` option added in #19074. In #19074 (targeted for 2.17), it defaults `lambdex` (retaining the current code paths). This PR flips the default to the new option `zip`, which keys into the functionality above. I've tried to keep the non-lambdex implementation generally separate to the lambdex one, rather than reusing all of the code that happens to be common currently, because it'd make sense to deprecate/remove the lambdex functionality and thus I feel it's best for this new functionality to be mostly a fresh start. This PR's commits can be reviewed independently. I _think_ this is an acceptable MVP for this functionality, but there's various bits of follow-up: - add a warning about `files` being loaded into these packages, which has been temporarily lost (#19027) - adjust documentation #19067 - other improvements like #18195 and #18880 - improve performance, e.g. potentially `pex3 venv create ...` could use the lock file and sources to directly compute the appropriate files, without having to materialise a normal pex first This is a re-doing of #19022 with a simpler approach to deprecation, as discussed in #19074 (comment) and #19032 (comment). The phasing will be: | release | supports lambdex? | supports zip? | default layout | deprecation warnings | |---|---|---|---|---| | 2.17 (this PR) | ✅ | ✅ | lambdex | if `layout = "lambdex"` is implicit, tell people to set it: recommend `zip`, but allow `lambdex` if they have to | | 2.18 | ✅ | ✅ | zip | if `layout = "lambdex"` is set at all, tell people to remove it and switch to `zip` | | 2.19 | ❌ | ✅ | zip | none, migration over (or maybe just about removing the `[lambdex]` section entirely) |

…d#19076) This fixes pantsbuild#18879 by allowing the `python_awslambda` and `python_google_cloud_function` FaaS artefacts to be generated in "simple" format, using the `pex3 venv create --layout=flat-zipped` functionality recently added in PEX 2.1.135 (https://github.com/pantsbuild/pex/releases/tag/v2.1.135). This format is just: put everything at the top-level. For instance, the zip contains `cowsay/__init__.py` etc., rather than `.deps/cowsay-....whl`. This avoids the need to do the dynamic PEX initialisation/venv creation. This shifts the dynamic dependency computation/extraction/layout from run-time to build-time, relying on the FaaS environment to be generally consistent. It shouldn't change what actually happens after initialisation. This can: - reduce cold-starts noticeably: for instance, some of our lambdas spend 1s doing PEX/Lambdex start up. - reduce package size somewhat (the PEX `.bootstrap/` folder seems to be about 2MB uncompressed, ~1MB compressed). - increase build times. For instance, for one Python 3.9 Lambda in our codebase: | metric | before | after | |---|---|---| | init time on cold start | 2.3-2.5s | 1.3-1.4s (-1s) | | compressed size | 24.6MB | 23.8MB (-0.8MB) | | uncompressed size | 117.8MB | 115.8MB (-2.0MB) | | PEX-construction build time | ~5s | ~5s | | PEX-postprocessing build time | 0.14s | 4.8s | (The PEX-postprocessing time metric is specifically the time to run the `Setting up handler` (lambdex) or `Build python_awslambda` (`pex3 venv create`) process, computed by running `pants --keep-sandboxes=always package ...` for each layout, and then `hyperfine -r3 -w1 path/to/first/__run.sh path/to/second/__run.sh`. This _doesn't_ include the time to construct the input PEX, which is the same for both.) --- This functionality is driven by adding a new option to the `[lambdex].layout` option added in pantsbuild#19074. In pantsbuild#19074 (targeted for 2.17), it defaults `lambdex` (retaining the current code paths). This PR flips the default to the new option `zip`, which keys into the functionality above. I've tried to keep the non-lambdex implementation generally separate to the lambdex one, rather than reusing all of the code that happens to be common currently, because it'd make sense to deprecate/remove the lambdex functionality and thus I feel it's best for this new functionality to be mostly a fresh start. This PR's commits can be reviewed independently. I _think_ this is an acceptable MVP for this functionality, but there's various bits of follow-up: - add a warning about `files` being loaded into these packages, which has been temporarily lost (pantsbuild#19027) - adjust documentation pantsbuild#19067 - other improvements like pantsbuild#18195 and pantsbuild#18880 - improve performance, e.g. potentially `pex3 venv create ...` could use the lock file and sources to directly compute the appropriate files, without having to materialise a normal pex first This is a re-doing of pantsbuild#19022 with a simpler approach to deprecation, as discussed in pantsbuild#19074 (comment) and pantsbuild#19032 (comment). The phasing will be: | release | supports lambdex? | supports zip? | default layout | deprecation warnings | |---|---|---|---|---| | 2.17 (this PR) | ✅ | ✅ | lambdex | if `layout = "lambdex"` is implicit, tell people to set it: recommend `zip`, but allow `lambdex` if they have to | | 2.18 | ✅ | ✅ | zip | if `layout = "lambdex"` is set at all, tell people to remove it and switch to `zip` | | 2.19 | ❌ | ✅ | zip | none, migration over (or maybe just about removing the `[lambdex]` section entirely) |

…files-warning

src/python/pants/backend/python/goals/package_pex_binary.py

…ck of #19076) (#19120) This fixes #18879 by allowing the `python_awslambda` and `python_google_cloud_function` FaaS artefacts to be generated in "simple" format, using the `pex3 venv create --layout=flat-zipped` functionality recently added in PEX 2.1.135 (https://github.com/pantsbuild/pex/releases/tag/v2.1.135). This format is just: put everything at the top-level. For instance, the zip contains `cowsay/__init__.py` etc., rather than `.deps/cowsay-....whl`. This avoids the need to do the dynamic PEX initialisation/venv creation. This shifts the dynamic dependency computation/extraction/layout from run-time to build-time, relying on the FaaS environment to be generally consistent. It shouldn't change what actually happens after initialisation. This can: - reduce cold-starts noticeably: for instance, some of our lambdas spend 1s doing PEX/Lambdex start up. - reduce package size somewhat (the PEX `.bootstrap/` folder seems to be about 2MB uncompressed, ~1MB compressed). - increase build times. For instance, for one Python 3.9 Lambda in our codebase: | metric | before | after | |---|---|---| | init time on cold start | 2.3-2.5s | 1.3-1.4s (-1s) | | compressed size | 24.6MB | 23.8MB (-0.8MB) | | uncompressed size | 117.8MB | 115.8MB (-2.0MB) | | PEX-construction build time | ~5s | ~5s | | PEX-postprocessing build time | 0.14s | 4.8s | (The PEX-postprocessing time metric is specifically the time to run the `Setting up handler` (lambdex) or `Build python_awslambda` (`pex3 venv create`) process, computed by running `pants --keep-sandboxes=always package ...` for each layout, and then `hyperfine -r3 -w1 path/to/first/__run.sh path/to/second/__run.sh`. This _doesn't_ include the time to construct the input PEX, which is the same for both.) --- This functionality is driven by adding a new option to the `[lambdex].layout` option added in #19074. In #19074 (targeted for 2.17), it defaults `lambdex` (retaining the current code paths). This PR flips the default to the new option `zip`, which keys into the functionality above. I've tried to keep the non-lambdex implementation generally separate to the lambdex one, rather than reusing all of the code that happens to be common currently, because it'd make sense to deprecate/remove the lambdex functionality and thus I feel it's best for this new functionality to be mostly a fresh start. This PR's commits can be reviewed independently. I _think_ this is an acceptable MVP for this functionality, but there's various bits of follow-up: - add a warning about `files` being loaded into these packages, which has been temporarily lost (#19027) - adjust documentation #19067 - other improvements like #18195 and #18880 - improve performance, e.g. potentially `pex3 venv create ...` could use the lock file and sources to directly compute the appropriate files, without having to materialise a normal pex first This is a re-doing of #19022 with a simpler approach to deprecation, as discussed in #19074 (comment) and #19032 (comment). The phasing will be: | release | supports lambdex? | supports zip? | default layout | deprecation warnings | |---|---|---|---|---| | 2.17 (this PR) | ✅ | ✅ | lambdex | if `layout = "lambdex"` is implicit, tell people to set it: recommend `zip`, but allow `lambdex` if they have to | | 2.18 | ✅ | ✅ | zip | if `layout = "lambdex"` is set at all, tell people to remove it and switch to `zip` | | 2.19 | ❌ | ✅ | zip | none, migration over (or maybe just about removing the `[lambdex]` section entirely) |

There's several places that check whether a PEX transitively depends on any `files()` targets, because those are (currently) not included in the PEX. This moves the warning into the "pex from targets" builder, to not have to reimplement it everywhere. This is follow up to #19022, and restores the warning for `layout="zip"`, since I didn't implement it there.

…19148) There's several places that check whether a PEX transitively depends on any `files()` targets, because those are (currently) not included in the PEX. This moves the warning into the "pex from targets" builder, to not have to reimplement it everywhere. This is follow up to #19022, and restores the warning for `layout="zip"`, since I didn't implement it there. Co-authored-by: Huon Wilson <[email protected]>

Internal changes: * Fix TypedDict construction ([pantsbuild#19166](pantsbuild#19166)) * Have package_dists.InvalidEntryPoint subclass InvalidFieldException ([pantsbuild#19149](pantsbuild#19149)) * Check for `CI=true`, not `CI=1` for cherry-picking ([pantsbuild#19141](pantsbuild#19141)) * Set up remote caching in CI ([pantsbuild#19144](pantsbuild#19144)) * refactor(internal): adjust pants_venv venv_dir calculation ([pantsbuild#19140](pantsbuild#19140)) * Prepare `2.16.0rc4`. ([pantsbuild#19146](pantsbuild#19146)) * Warn about `files()` in PEXes in one place ([pantsbuild#19027](pantsbuild#19027)) * Drop our explicit dep on `syn` ([pantsbuild#19134](pantsbuild#19134)) * Fix auto-cherry-picker's merge-conflicts ([pantsbuild#19125](pantsbuild#19125)) * Use explicit depths on fetches when running cherry-picks in CI ([pantsbuild#19127](pantsbuild#19127)) * Make determine_python() pin to 3.9 ([pantsbuild#19116](pantsbuild#19116)) * Upgrade to `actions/checkout@v3` ([pantsbuild#19112](pantsbuild#19112)) * Run the cherry pick script with `-x` for debugging ([pantsbuild#19095](pantsbuild#19095)) * Add more linux x86 test CI shards ([pantsbuild#19099](pantsbuild#19099)) * Use a trait for remote ByteStore network implementation ([pantsbuild#19050](pantsbuild#19050)) * Increase the logging of release version file publishing. ([pantsbuild#19092](pantsbuild#19092)) * Remove the Toolchain plugin ([pantsbuild#18978](pantsbuild#18978)) * Fix bad merge of pantsbuild#18829. ([pantsbuild#19080](pantsbuild#19080)) * Revert "Implement layout="zip" for Lambda/GCF, skipping lambdex" ([pantsbuild#19071](pantsbuild#19071)) * Remove unnecessary interpreter constraint config. ([pantsbuild#19065](pantsbuild#19065)) * Use hand-written change detection for PR/push job trimming ([pantsbuild#19045](pantsbuild#19045)) * Do not build wheels on push, since that is now accomplished by the release job. ([pantsbuild#19063](pantsbuild#19063)) * Additional release job publishing fixes ([pantsbuild#19058](pantsbuild#19058)) * Change CI trigger to includelist `main` and `2.*.x` ([pantsbuild#19059](pantsbuild#19059)) * Release doc and script fixes ([pantsbuild#19043](pantsbuild#19043)) * Update cherry-picker to use `pull_request_target` ([pantsbuild#19048](pantsbuild#19048)) * Prepare `2.16.0rc3`. ([pantsbuild#19044](pantsbuild#19044)) * Prepare `2.15.1`. ([pantsbuild#19042](pantsbuild#19042)) * Fixup welcome-newcomers.yaml ([pantsbuild#19039](pantsbuild#19039))

Internal changes: * Fix TypedDict construction ([#19166](#19166)) * Have package_dists.InvalidEntryPoint subclass InvalidFieldException ([#19149](#19149)) * Check for `CI=true`, not `CI=1` for cherry-picking ([#19141](#19141)) * Set up remote caching in CI ([#19144](#19144)) * refactor(internal): adjust pants_venv venv_dir calculation ([#19140](#19140)) * Prepare `2.16.0rc4`. ([#19146](#19146)) * Warn about `files()` in PEXes in one place ([#19027](#19027)) * Drop our explicit dep on `syn` ([#19134](#19134)) * Fix auto-cherry-picker's merge-conflicts ([#19125](#19125)) * Use explicit depths on fetches when running cherry-picks in CI ([#19127](#19127)) * Make determine_python() pin to 3.9 ([#19116](#19116)) * Upgrade to `actions/checkout@v3` ([#19112](#19112)) * Run the cherry pick script with `-x` for debugging ([#19095](#19095)) * Add more linux x86 test CI shards ([#19099](#19099)) * Use a trait for remote ByteStore network implementation ([#19050](#19050)) * Increase the logging of release version file publishing. ([#19092](#19092)) * Remove the Toolchain plugin ([#18978](#18978)) * Fix bad merge of #18829. ([#19080](#19080)) * Revert "Implement layout="zip" for Lambda/GCF, skipping lambdex" ([#19071](#19071)) * Remove unnecessary interpreter constraint config. ([#19065](#19065)) * Use hand-written change detection for PR/push job trimming ([#19045](#19045)) * Do not build wheels on push, since that is now accomplished by the release job. ([#19063](#19063)) * Additional release job publishing fixes ([#19058](#19058)) * Change CI trigger to includelist `main` and `2.*.x` ([#19059](#19059)) * Release doc and script fixes ([#19043](#19043)) * Update cherry-picker to use `pull_request_target` ([#19048](#19048)) * Prepare `2.16.0rc3`. ([#19044](#19044)) * Prepare `2.15.1`. ([#19042](#19042)) * Fixup welcome-newcomers.yaml ([#19039](#19039))

huonw mentioned this pull request May 17, 2023

Implement layout="zip" for Lambda/GCF, skipping lambdex #19022

Merged

Warn about files() in PEXes in one place

bc820cd

huonw force-pushed the feature/centralise-files-warning branch from 337bcc5 to bc820cd Compare May 20, 2023 02:11

huonw added the category:internal CI, fixes for not-yet-released features, etc. label May 20, 2023

huonw commented May 20, 2023

View reviewed changes

src/python/pants/backend/python/util_rules/pex_from_targets.py Outdated Show resolved Hide resolved

huonw marked this pull request as ready for review May 20, 2023 02:24

huonw requested review from kaos, benjyw and cognifloyd May 20, 2023 02:24

Restore target alias into error message

dcbf1c5

huonw mentioned this pull request May 21, 2023

Implement layout="zip" for Lambda/GCF, deprecating lambdex #19076

Merged

WorkerPants mentioned this pull request May 23, 2023

Implement layout="zip" for Lambda/GCF, deprecating lambdex (Cherry-pick of #19076) #19119

Closed

huonw mentioned this pull request May 23, 2023

Implement layout="zip" for Lambda/GCF, deprecating lambdex (Cherry-pick of #19076) #19120

Merged

huonw added this to the 2.17.x milestone May 23, 2023

huonw added the needs-cherrypick label May 23, 2023

Merge remote-tracking branch 'upstream/main' into feature/centralise-…

80e8489

…files-warning

cognifloyd approved these changes May 23, 2023

View reviewed changes

src/python/pants/backend/python/goals/package_pex_binary.py Outdated Show resolved Hide resolved

Review: outdated comment, unnecessary variable

f77fb9d

cognifloyd merged commit a963b6d into pantsbuild:main May 25, 2023

WorkerPants removed the needs-cherrypick label May 25, 2023

huonw deleted the feature/centralise-files-warning branch May 25, 2023 05:03

illicitonion mentioned this pull request May 26, 2023

Prep 2.18.0.dev0 #19167

Merged

stuhood mentioned this pull request Jun 1, 2023

Prepare 2.17.0rc0. #19226

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warn about `files()` in PEXes in one place #19027

Warn about `files()` in PEXes in one place #19027

huonw commented May 17, 2023 •

edited

Loading

Warn about files() in PEXes in one place #19027

Warn about files() in PEXes in one place #19027

Conversation

huonw commented May 17, 2023 • edited Loading

Warn about `files()` in PEXes in one place #19027

Warn about `files()` in PEXes in one place #19027

huonw commented May 17, 2023 •

edited

Loading