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.

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

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

Merged
merged 10 commits into from
May 20, 2023
45 changes: 36 additions & 9 deletions src/python/pants/backend/awslambda/python/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@
PythonAwsLambdaIncludeRequirements,
PythonAwsLambdaRuntime,
)
from pants.backend.python.util_rules import pex_from_targets
from pants.backend.python.util_rules.faas import BuildLambdexRequest, PythonFaaSCompletePlatforms
from pants.backend.python.util_rules.faas import (
BuildLambdexRequest,
BuildPythonFaaSRequest,
PythonFaaSCompletePlatforms,
PythonFaaSLayout,
PythonFaaSLayoutField,
)
from pants.backend.python.util_rules.faas import rules as faas_rules
from pants.core.goals.package import BuiltPackage, OutputPathField, PackageFieldSet
from pants.core.util_rules.environments import EnvironmentField
from pants.engine.rules import Get, collect_rules, rule
Expand All @@ -33,27 +39,48 @@ class PythonAwsLambdaFieldSet(PackageFieldSet):
complete_platforms: PythonFaaSCompletePlatforms
output_path: OutputPathField
environment: EnvironmentField
layout: PythonFaaSLayoutField


@rule(desc="Create Python AWS Lambda", level=LogLevel.DEBUG)
async def package_python_awslambda(
field_set: PythonAwsLambdaFieldSet,
) -> BuiltPackage:
layout = PythonFaaSLayout(field_set.layout.value)

if layout is PythonFaaSLayout.LAMBDEX:
return await Get(
BuiltPackage,
BuildLambdexRequest(
address=field_set.address,
target_name=PythonAWSLambda.alias,
complete_platforms=field_set.complete_platforms,
runtime=field_set.runtime,
handler=field_set.handler,
output_path=field_set.output_path,
include_requirements=field_set.include_requirements.value,
script_handler=None,
script_module=None,
# The AWS-facing handler function is always lambdex_handler.handler, which is the
# wrapper injected by lambdex that manages invocation of the actual handler.
handler_log_message="lambdex_handler.handler",
),
)

return await Get(
BuiltPackage,
BuildLambdexRequest(
BuildPythonFaaSRequest(
address=field_set.address,
target_name=PythonAWSLambda.alias,
complete_platforms=field_set.complete_platforms,
runtime=field_set.runtime,
handler=field_set.handler,
layout=layout,
output_path=field_set.output_path,
include_requirements=field_set.include_requirements.value,
script_handler=None,
script_module=None,
# The AWS-facing handler function is always lambdex_handler.handler, which is the
# wrapper injected by lambdex that manages invocation of the actual handler.
handler_log_message="lambdex_handler.handler",
# this isn't built or run via lambdex, but the name is arbitrary, so we stick with the
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a good opportunity to get rid of this name, since we're churning anyway. It'll be harder to do later.

Copy link
Contributor

Choose a reason for hiding this comment

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

But also, are we sure we want this at all? Why not use the handler name the user configured directly?

Copy link
Contributor Author

@huonw huonw May 17, 2023

Choose a reason for hiding this comment

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

But also, are we sure we want this at all? Why not use the handler name the user configured directly?

Yes, I think so. The BUILD file target must contain the entry point (at least the file/module part of entry_point="path/to.py:handler"), in order to compute dependencies/build the package, and it's annoying to have to carefully reproduce that elsewhere (handler: 'parent.modules.path.to.handler' in templates/IaC code). Synthesising a fixed module avoids that, and is all info pants already knows.

For instance, in our repo, we use AWS CDK, and have a generic PythonLambda construct where the basic "get the code running" config is just pointing it to the package (e.g. foo.bar/target.zip), because it can just set handler: 'lambdex_handler.handler' globally, not requiring customisation per lambda. I think this is really nice!

(There's obviously often per-lambda config required like any extra env vars or permissions, but CDK/the infra configuration is the source of truth there, it's not just directly duplicated from the BUILD files.)

I think this is a good opportunity to get rid of this name, since we're churning anyway. It'll be harder to do later.

Sure, I'm happy to choose another name, e.g. some options:

I think we can also make this customisable in future (including the ability to turn it off), e.g. synthetic_handler_module="my_custom_name" or synthetic_handler_module=None, if required/desired.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've gone with lambda_function, but more than happy to change. (Thought of another option too: main to be consistent with GCF.)

# 'tradition' to reduce churn.
reexported_handler_module="lambdex_handler",
),
)

Expand All @@ -62,5 +89,5 @@ def rules():
return [
*collect_rules(),
UnionRule(PackageFieldSet, PythonAwsLambdaFieldSet),
*pex_from_targets.rules(),
*faas_rules(),
]
69 changes: 67 additions & 2 deletions src/python/pants/backend/awslambda/python/rules_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def complete_platform(rule_runner: PythonRuleRunner) -> bytes:
"major_minor_interpreter",
all_major_minor_python_versions(Lambdex.default_interpreter_constraints),
)
def test_create_hello_world_lambda(
def test_create_hello_world_lambda_with_lambdex(
rule_runner: PythonRuleRunner, major_minor_interpreter: str, complete_platform: str, caplog
) -> None:
rule_runner.write_files(
Expand Down Expand Up @@ -197,7 +197,7 @@ def handler(event, context):
), "Using include_requirements=False should exclude third-party deps"


def test_warn_files_targets(rule_runner: PythonRuleRunner, caplog) -> None:
def test_warn_files_targets_with_lambdex(rule_runner: PythonRuleRunner, caplog) -> None:
rule_runner.write_files(
{
"assets/f.txt": "",
Expand Down Expand Up @@ -257,3 +257,68 @@ def handler(event, context):
assert "assets/f.txt:files" in caplog.text
assert "assets:relocated" in caplog.text
assert "assets:resources" not in caplog.text


def test_create_hello_world_lambda(rule_runner: PythonRuleRunner) -> None:
rule_runner.write_files(
{
"src/python/foo/bar/hello_world.py": dedent(
"""
import mureq

def handler(event, context):
print('Hello, World!')
"""
),
"src/python/foo/bar/BUILD": dedent(
"""
python_requirement(name="mureq", requirements=["mureq==0.2"])
python_sources()

python_awslambda(
name='lambda',
handler='foo.bar.hello_world:handler',
runtime="python3.7",
layout='zip',
)
python_awslambda(
name='slimlambda',
include_requirements=False,
handler='foo.bar.hello_world:handler',
runtime="python3.7",
layout='zip',
)
"""
),
}
)

zip_file_relpath, content = create_python_awslambda(
rule_runner,
Address("src/python/foo/bar", target_name="lambda"),
expected_extra_log_lines=(" Handler: lambdex_handler.handler",),
)
assert "src.python.foo.bar/lambda.zip" == zip_file_relpath

zipfile = ZipFile(BytesIO(content))
names = set(zipfile.namelist())
assert "mureq/__init__.py" in names
assert "foo/bar/hello_world.py" in names
assert (
zipfile.read("lambdex_handler.py") == b"from foo.bar.hello_world import handler as handler"
)

zip_file_relpath, content = create_python_awslambda(
rule_runner,
Address("src/python/foo/bar", target_name="slimlambda"),
expected_extra_log_lines=(" Handler: lambdex_handler.handler",),
)
assert "src.python.foo.bar/slimlambda.zip" == zip_file_relpath

zipfile = ZipFile(BytesIO(content))
names = set(zipfile.namelist())
assert "mureq/__init__.py" not in names
assert "foo/bar/hello_world.py" in names
assert (
zipfile.read("lambdex_handler.py") == b"from foo.bar.hello_world import handler as handler"
)
2 changes: 2 additions & 0 deletions src/python/pants/backend/awslambda/python/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
PythonFaaSCompletePlatforms,
PythonFaaSDependencies,
PythonFaaSHandlerField,
PythonFaaSLayoutField,
PythonFaaSRuntimeField,
)
from pants.backend.python.util_rules.faas import rules as faas_rules
Expand Down Expand Up @@ -111,6 +112,7 @@ class PythonAWSLambda(Target):
PythonAwsLambdaRuntime,
PythonFaaSCompletePlatforms,
PythonResolveField,
PythonFaaSLayoutField,
EnvironmentField,
)
help = help_text(
Expand Down
53 changes: 42 additions & 11 deletions src/python/pants/backend/google_cloud_function/python/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@
PythonGoogleCloudFunctionRuntime,
PythonGoogleCloudFunctionType,
)
from pants.backend.python.util_rules import pex_from_targets
from pants.backend.python.util_rules.faas import BuildLambdexRequest, PythonFaaSCompletePlatforms
from pants.backend.python.util_rules.faas import (
BuildLambdexRequest,
BuildPythonFaaSRequest,
PythonFaaSCompletePlatforms,
PythonFaaSLayout,
PythonFaaSLayoutField,
)
from pants.backend.python.util_rules.faas import rules as faas_rules
from pants.core.goals.package import BuiltPackage, OutputPathField, PackageFieldSet
from pants.core.util_rules.environments import EnvironmentField
from pants.engine.rules import Get, collect_rules, rule
Expand All @@ -33,33 +39,58 @@ class PythonGoogleCloudFunctionFieldSet(PackageFieldSet):
type: PythonGoogleCloudFunctionType
output_path: OutputPathField
environment: EnvironmentField
layout: PythonFaaSLayoutField


@rule(desc="Create Python Google Cloud Function", level=LogLevel.DEBUG)
async def package_python_google_cloud_function(
field_set: PythonGoogleCloudFunctionFieldSet,
) -> BuiltPackage:
layout = PythonFaaSLayout(field_set.layout.value)
if layout is PythonFaaSLayout.LAMBDEX:
return await Get(
BuiltPackage,
BuildLambdexRequest(
address=field_set.address,
target_name=PythonGoogleCloudFunction.alias,
complete_platforms=field_set.complete_platforms,
runtime=field_set.runtime,
handler=field_set.handler,
output_path=field_set.output_path,
include_requirements=True,
# The GCP-facing handler function is always `main.handler` (We pass `-M main.py -H handler` to
# Lambdex to ensure this), which is the wrapper injected by Lambdex that manages invocation of
# the actual user-supplied handler function. This arrangement works well since GCF assumes the
# handler function is housed in `main.py` in the root of the zip (you can re-direct this by
# setting a `GOOGLE_FUNCTION_SOURCE` Google Cloud build environment variable; e.g.:
# `gcloud functions deploy {--build-env-vars-file,--set-build-env-vars}`, but it's non-trivial
# to do this right or with intended effect) and the handler name you configure GCF with is just
# the unqualified function name, which we log here.
script_handler="handler",
script_module="main.py",
handler_log_message="handler",
),
)

return await Get(
BuiltPackage,
BuildLambdexRequest(
BuildPythonFaaSRequest(
address=field_set.address,
target_name=PythonGoogleCloudFunction.alias,
complete_platforms=field_set.complete_platforms,
runtime=field_set.runtime,
handler=field_set.handler,
layout=layout,
output_path=field_set.output_path,
include_requirements=True,
# The GCP-facing handler function is always `main.handler` (We pass `-M main.py -H handler` to
# Lambdex to ensure this), which is the wrapper injected by Lambdex that manages invocation of
# the actual user-supplied handler function. This arrangement works well since GCF assumes the
# handler function is housed in `main.py` in the root of the zip (you can re-direct this by
# GCP requires "Your main file must be named main.py"
# https://cloud.google.com/functions/docs/writing#directory-structure-python
# setting a `GOOGLE_FUNCTION_SOURCE` Google Cloud build environment variable; e.g.:
# `gcloud functions deploy {--build-env-vars-file,--set-build-env-vars}`, but it's non-trivial
# to do this right or with intended effect) and the handler name you configure GCF with is just
# the unqualified function name, which we log here.
script_handler="handler",
script_module="main.py",
handler_log_message="handler",
reexported_handler_module="main",
log_only_reexported_handler_func=True,
),
)

Expand All @@ -68,5 +99,5 @@ def rules():
return [
*collect_rules(),
UnionRule(PackageFieldSet, PythonGoogleCloudFunctionFieldSet),
*pex_from_targets.rules(),
*faas_rules(),
]
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def complete_platform(rule_runner: PythonRuleRunner) -> bytes:
"major_minor_interpreter",
all_major_minor_python_versions(Lambdex.default_interpreter_constraints),
)
def test_create_hello_world_lambda(
def test_create_hello_world_lambda_with_lambdex(
rule_runner: PythonRuleRunner, major_minor_interpreter: str, complete_platform: str, caplog
) -> None:
rule_runner.write_files(
Expand Down Expand Up @@ -243,3 +243,45 @@ def handler(event, context):
assert "assets/f.txt:files" in caplog.text
assert "assets:relocated" in caplog.text
assert "assets:resources" not in caplog.text


def test_create_hello_world_gcf(rule_runner: PythonRuleRunner) -> None:
rule_runner.write_files(
{
"src/python/foo/bar/hello_world.py": dedent(
"""
import mureq

def handler(event, context):
print('Hello, World!')
"""
),
"src/python/foo/bar/BUILD": dedent(
"""
python_requirement(name="mureq", requirements=["mureq==0.2"])
python_sources()

python_google_cloud_function(
name='gcf',
handler='foo.bar.hello_world:handler',
runtime="python37",
layout='zip',
type='event',
)
"""
),
}
)

zip_file_relpath, content = create_python_google_cloud_function(
rule_runner,
Address("src/python/foo/bar", target_name="gcf"),
expected_extra_log_lines=(" Handler: handler",),
)
assert "src.python.foo.bar/gcf.zip" == zip_file_relpath

zipfile = ZipFile(BytesIO(content))
names = set(zipfile.namelist())
assert "mureq/__init__.py" in names
assert "foo/bar/hello_world.py" in names
assert zipfile.read("main.py") == b"from foo.bar.hello_world import handler as handler"
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
PythonFaaSCompletePlatforms,
PythonFaaSDependencies,
PythonFaaSHandlerField,
PythonFaaSLayoutField,
PythonFaaSRuntimeField,
)
from pants.backend.python.util_rules.faas import rules as faas_rules
Expand Down Expand Up @@ -111,6 +112,7 @@ class PythonGoogleCloudFunction(Target):
PythonFaaSCompletePlatforms,
PythonGoogleCloudFunctionType,
PythonResolveField,
PythonFaaSLayoutField,
EnvironmentField,
)
help = help_text(
Expand Down
Loading