Skip to content
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.

Sign up for GitHub

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

Add warning when building AWS Lambdas and Google Cloud Functions on macOS. #13790

Merged
merged 7 commits into from
Dec 15, 2021
Merged

Add warning when building AWS Lambdas and Google Cloud Functions on macOS. #13790

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
de5eaa6
Add warning when building lambdas on macOS.
Dec 3, 2021
a810845
Address PR feedback. Add warning for Google Cloud Functions.
Dec 10, 2021
975cb2e
Update src/python/pants/backend/awslambda/python/rules.py
riisi Dec 14, 2021
4cd2183
Update src/python/pants/backend/google_cloud_function/python/rules.py
riisi Dec 14, 2021
48e615f
Merge branch 'main' of github.com:pantsbuild/pants into fix/lambda-error
Eric-Arellano Dec 14, 2021
05a1c1c
Fix fmt
Eric-Arellano Dec 14, 2021
6b5a89e
Fix MyPy not liking shadowing of variable name
Eric-Arellano Dec 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/python/pants/backend/awslambda/python/rules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
PackageFieldSet,
)
from pants.core.target_types import FileSourceField
from pants.engine.platform import Platform
from pants.engine.process import ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import (
Expand Down Expand Up @@ -54,6 +55,17 @@ class PythonAwsLambdaFieldSet(PackageFieldSet):
async def package_python_awslambda(
field_set: PythonAwsLambdaFieldSet, lambdex: Lambdex, union_membership: UnionMembership
) -> BuiltPackage:

if Platform.is_macos:
riisi marked this conversation as resolved.
Show resolved Hide resolved
logger.warning(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be useful information if the packing fails, but it's otherwise continual noise every time this is run and packaging succeeds. How about switching to a FallibleProcessResult and then conditioning this warning on both macOS and process failure?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that would be a superior experience.

Per discussion at #11941, implementing it is trickier. We need to

  1. Add a FalliblePex type
  2. Allow augmenting a ProcessExecutionFailure with additional details.

So I suggested that using a warning is a good first start to get some progress.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Erm. Ok. I'm not a fan of bridging the gap with noise, but looks like others are.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #13865 to track that second task.

"AWS Lambdas built on macOS may fail to build. If your lambda uses any third-party"
" dependencies without binary wheels (bdist) for Linux available, it will fail to"
" build. If this happens, you will either need to update your dependencies to only use"
" dependencies with pre-built wheels, or find a Linux environment to run ./pants"
" package. (See https://realpython.com/python-wheels/ for more about wheels.)\n\n(If"
" the build does not raise an exception, it's safe to use macOS.)"
)

output_filename = field_set.output_path.value_or_default(
# Lambdas typically use the .zip suffix, so we use that instead of .pex.
file_ending="zip",
Expand Down
7 changes: 6 additions & 1 deletion src/python/pants/backend/awslambda/python/rules_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from __future__ import annotations

import sys
from io import BytesIO
from textwrap import dedent
from zipfile import ZipFile
Expand Down Expand Up @@ -72,7 +73,9 @@ def create_python_awslambda(
"major_minor_interpreter",
all_major_minor_python_versions(Lambdex.default_interpreter_constraints),
)
def test_create_hello_world_lambda(rule_runner: RuleRunner, major_minor_interpreter: str) -> None:
def test_create_hello_world_lambda(
rule_runner: RuleRunner, major_minor_interpreter: str, caplog
) -> None:
rule_runner.write_files(
{
"src/python/foo/bar/hello_world.py": dedent(
Expand Down Expand Up @@ -105,6 +108,8 @@ def handler(event, context):
names = set(zipfile.namelist())
assert "lambdex_handler.py" in names
assert "foo/bar/hello_world.py" in names
if sys.platform == "darwin":
riisi marked this conversation as resolved.
Show resolved Hide resolved
assert "AWS Lambdas built on macOS may fail to build." in caplog.text


def test_warn_files_targets(rule_runner: RuleRunner, caplog) -> None:
Expand Down
12 changes: 12 additions & 0 deletions src/python/pants/backend/google_cloud_function/python/rules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
PackageFieldSet,
)
from pants.core.target_types import FileSourceField
from pants.engine.platform import Platform
from pants.engine.process import ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
from pants.engine.target import (
Expand Down Expand Up @@ -58,6 +59,17 @@ async def package_python_google_cloud_function(
lambdex: Lambdex,
union_membership: UnionMembership,
) -> BuiltPackage:

if Platform.is_macos:
riisi marked this conversation as resolved.
Show resolved Hide resolved
logger.warning(
"Google Cloud Functions built on macOS may fail to build. If your function uses any"
" third-party dependencies without binary wheels (bdist) for Linux available, it will"
" fail to build. If this happens, you will either need to update your dependencies to"
" only use dependencies with pre-built wheels, or find a Linux environment to run"
" ./pants package. (See https://realpython.com/python-wheels/ for more about"
" wheels.)\n\n(If the build does not raise an exception, it's safe to use macOS.)"
)

output_filename = field_set.output_path.value_or_default(
# Cloud Functions typically use the .zip suffix, so we use that instead of .pex.
file_ending="zip",
Expand Down
7 changes: 6 additions & 1 deletion src/python/pants/backend/google_cloud_function/python/rules_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from __future__ import annotations

import sys
from io import BytesIO
from textwrap import dedent
from zipfile import ZipFile
Expand Down Expand Up @@ -81,7 +82,9 @@ def create_python_google_cloud_function(
"major_minor_interpreter",
all_major_minor_python_versions(Lambdex.default_interpreter_constraints),
)
def test_create_hello_world_lambda(rule_runner: RuleRunner, major_minor_interpreter: str) -> None:
def test_create_hello_world_lambda(
rule_runner: RuleRunner, major_minor_interpreter: str, caplog
) -> None:
rule_runner.write_files(
{
"src/python/foo/bar/hello_world.py": dedent(
Expand Down Expand Up @@ -115,6 +118,8 @@ def handler(event, context):
names = set(zipfile.namelist())
assert "main.py" in names
assert "foo/bar/hello_world.py" in names
if sys.platform == "darwin":
assert "Google Cloud Functions built on macOS may fail to build." in caplog.text


def test_warn_files_targets(rule_runner: RuleRunner, caplog) -> None:
Expand Down