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

Clarify runtime vs. complete_platforms for serverless. #18001

Merged
merged 1 commit into from
Jan 15, 2023
Merged

Clarify runtime vs. complete_platforms for serverless. #18001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 13 additions & 3 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 @@ -7,14 +7,14 @@
from dataclasses import dataclass

from pants.backend.awslambda.python.target_types import (
PythonAwsLambdaCompletePlatforms,
PythonAwsLambdaHandlerField,
PythonAwsLambdaIncludeRequirements,
PythonAwsLambdaRuntime,
ResolvedPythonAwsHandler,
ResolvePythonAwsHandlerRequest,
)
from pants.backend.python.subsystems.lambdex import Lambdex
from pants.backend.python.target_types import PexCompletePlatformsField
from pants.backend.python.util_rules import pex_from_targets
from pants.backend.python.util_rules.pex import (
CompletePlatforms,
Expand All @@ -32,6 +32,7 @@
PackageFieldSet,
)
from pants.core.target_types import FileSourceField
from pants.engine.addresses import UnparsedAddressInputs
from pants.engine.platform import Platform
from pants.engine.process import ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
Expand All @@ -55,10 +56,19 @@ class PythonAwsLambdaFieldSet(PackageFieldSet):
handler: PythonAwsLambdaHandlerField
include_requirements: PythonAwsLambdaIncludeRequirements
runtime: PythonAwsLambdaRuntime
complete_platforms: PexCompletePlatformsField
complete_platforms: PythonAwsLambdaCompletePlatforms
output_path: OutputPathField


@rule
async def digest_complete_platforms(
complete_platforms: PythonAwsLambdaCompletePlatforms,
) -> CompletePlatforms:
return await Get(
CompletePlatforms, UnparsedAddressInputs, complete_platforms.to_unparsed_address_inputs()
)


@rule(desc="Create Python AWS Lambda", level=LogLevel.DEBUG)
async def package_python_awslambda(
field_set: PythonAwsLambdaFieldSet,
Expand Down Expand Up @@ -110,7 +120,7 @@ async def package_python_awslambda(
)

complete_platforms = await Get(
CompletePlatforms, PexCompletePlatformsField, field_set.complete_platforms
CompletePlatforms, PythonAwsLambdaCompletePlatforms, field_set.complete_platforms
)

pex_request = PexFromTargetsRequest(
Expand Down
19 changes: 18 additions & 1 deletion src/python/pants/backend/awslambda/python/target_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,11 @@ class PythonAwsLambdaRuntime(StringField):
"""
The identifier of the AWS Lambda runtime to target (pythonX.Y).
See https://docs.aws.amazon.com/lambda/latest/dg/lambda-python.html.

In general you'll want to define either a `runtime` or one `complete_platforms` but not
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens today if you specify both? I assume we'd want to error? And wouldn't that error message be sufficient??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, like any other PEX, multiplatform is supported and all platforms and complete_platforms are additive. So you can make a PEX that can be deployed to >1 lambda runtime. A separate step could be the whole deprecation procedure to restrict this down to one, which would include transitioning to platform XOR complete_platforms and making complete_platforms singular.

both. Specifying a `runtime` is simpler, but less accurate. If you have issues either
packaging the AWS Lambda PEX or running it as a deployed AWS Lambda function, you should try
using `complete_platforms` instead.
"""
)

Expand Down Expand Up @@ -273,6 +278,18 @@ def to_interpreter_version(self) -> Optional[Tuple[int, int]]:
return int(mo.group("major")), int(mo.group("minor"))


class PythonAwsLambdaCompletePlatforms(PexCompletePlatformsField):
help = softwrap(
f"""
{PexCompletePlatformsField.help}

N.B.: If specifying `complete_platforms` to work around packaging failures encountered when
using the `runtime` field, ensure you delete the `runtime` field from your
`python_awslambda` target.
"""
)


class PythonAWSLambda(Target):
alias = "python_awslambda"
core_fields = (
Expand All @@ -282,7 +299,7 @@ class PythonAWSLambda(Target):
PythonAwsLambdaHandlerField,
PythonAwsLambdaIncludeRequirements,
PythonAwsLambdaRuntime,
PexCompletePlatformsField,
PythonAwsLambdaCompletePlatforms,
PythonResolveField,
)
help = softwrap(
Expand Down
13 changes: 5 additions & 8 deletions src/python/pants/backend/awslambda/python/target_types_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,15 @@
from pants.backend.awslambda.python.target_types import (
InferPythonLambdaHandlerDependency,
PythonAWSLambda,
PythonAwsLambdaCompletePlatforms,
PythonAwsLambdaHandlerField,
PythonAwsLambdaRuntime,
PythonLambdaHandlerDependencyInferenceFieldSet,
ResolvedPythonAwsHandler,
ResolvePythonAwsHandlerRequest,
)
from pants.backend.awslambda.python.target_types import rules as target_type_rules
from pants.backend.python.target_types import (
PexCompletePlatformsField,
PythonRequirementTarget,
PythonSourcesGeneratorTarget,
)
from pants.backend.python.target_types import PythonRequirementTarget, PythonSourcesGeneratorTarget
from pants.backend.python.target_types_rules import rules as python_target_types_rules
from pants.build_graph.address import Address
from pants.core.target_types import FileTarget
Expand Down Expand Up @@ -280,17 +277,17 @@ def test_at_least_one_target_platform(rule_runner: RuleRunner) -> None:

runtime = rule_runner.get_target(Address("project", target_name="runtime"))
assert "python3.7" == runtime[PythonAwsLambdaRuntime].value
assert runtime[PexCompletePlatformsField].value is None
assert runtime[PythonAwsLambdaCompletePlatforms].value is None

complete_platforms = rule_runner.get_target(
Address("project", target_name="complete_platforms")
)
assert complete_platforms[PythonAwsLambdaRuntime].value is None
assert (":python37",) == complete_platforms[PexCompletePlatformsField].value
assert (":python37",) == complete_platforms[PythonAwsLambdaCompletePlatforms].value

both = rule_runner.get_target(Address("project", target_name="both"))
assert "python3.7" == both[PythonAwsLambdaRuntime].value
assert (":python37",) == both[PexCompletePlatformsField].value
assert (":python37",) == both[PythonAwsLambdaCompletePlatforms].value

with pytest.raises(
ExecutionError,
Expand Down
16 changes: 13 additions & 3 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 @@ -7,14 +7,14 @@
from dataclasses import dataclass

from pants.backend.google_cloud_function.python.target_types import (
PythonGoogleCloudFunctionCompletePlatforms,
PythonGoogleCloudFunctionHandlerField,
PythonGoogleCloudFunctionRuntime,
PythonGoogleCloudFunctionType,
ResolvedPythonGoogleHandler,
ResolvePythonGoogleHandlerRequest,
)
from pants.backend.python.subsystems.lambdex import Lambdex
from pants.backend.python.target_types import PexCompletePlatformsField
from pants.backend.python.util_rules import pex_from_targets
from pants.backend.python.util_rules.pex import (
CompletePlatforms,
Expand All @@ -32,6 +32,7 @@
PackageFieldSet,
)
from pants.core.target_types import FileSourceField
from pants.engine.addresses import UnparsedAddressInputs
from pants.engine.platform import Platform
from pants.engine.process import ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule
Expand All @@ -53,11 +54,20 @@ class PythonGoogleCloudFunctionFieldSet(PackageFieldSet):

handler: PythonGoogleCloudFunctionHandlerField
runtime: PythonGoogleCloudFunctionRuntime
complete_platforms: PexCompletePlatformsField
complete_platforms: PythonGoogleCloudFunctionCompletePlatforms
type: PythonGoogleCloudFunctionType
output_path: OutputPathField


@rule
async def digest_complete_platforms(
complete_platforms: PythonGoogleCloudFunctionCompletePlatforms,
) -> CompletePlatforms:
return await Get(
CompletePlatforms, UnparsedAddressInputs, complete_platforms.to_unparsed_address_inputs()
)


@rule(desc="Create Python Google Cloud Function", level=LogLevel.DEBUG)
async def package_python_google_cloud_function(
field_set: PythonGoogleCloudFunctionFieldSet,
Expand Down Expand Up @@ -102,7 +112,7 @@ async def package_python_google_cloud_function(
)

complete_platforms = await Get(
CompletePlatforms, PexCompletePlatformsField, field_set.complete_platforms
CompletePlatforms, PythonGoogleCloudFunctionCompletePlatforms, field_set.complete_platforms
)

pex_request = PexFromTargetsRequest(
Expand Down
19 changes: 18 additions & 1 deletion src/python/pants/backend/google_cloud_function/python/target_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,11 @@ class PythonGoogleCloudFunctionRuntime(StringField):
"""
The identifier of the Google Cloud Function runtime to target (pythonXY). See
https://cloud.google.com/functions/docs/concepts/python-runtime.

In general you'll want to define either a `runtime` or one `complete_platforms` but not
both. Specifying a `runtime` is simpler, but less accurate. If you have issues either
packaging the Google Cloud Function PEX or running it as a deployed Google Cloud Function,
you should try using `complete_platforms` instead.
"""
)

Expand All @@ -254,6 +259,18 @@ def to_interpreter_version(self) -> Optional[Tuple[int, int]]:
return int(mo.group("major")), int(mo.group("minor"))


class PythonGoogleCloudFunctionCompletePlatforms(PexCompletePlatformsField):
help = softwrap(
f"""
{PexCompletePlatformsField.help}

N.B.: If specifying `complete_platforms` to work around packaging failures encountered when
using the `runtime` field, ensure you delete the `runtime` field from your
`python_google_cloud_function` target.
"""
)


class GoogleCloudFunctionTypes(Enum):
EVENT = "event"
HTTP = "http"
Expand Down Expand Up @@ -281,7 +298,7 @@ class PythonGoogleCloudFunction(Target):
PythonGoogleCloudFunctionDependencies,
PythonGoogleCloudFunctionHandlerField,
PythonGoogleCloudFunctionRuntime,
PexCompletePlatformsField,
PythonGoogleCloudFunctionCompletePlatforms,
PythonGoogleCloudFunctionType,
PythonResolveField,
)
Expand Down
13 changes: 5 additions & 8 deletions src/python/pants/backend/google_cloud_function/python/target_types_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,14 @@
InferPythonCloudFunctionHandlerDependency,
PythonCloudFunctionHandlerInferenceFieldSet,
PythonGoogleCloudFunction,
PythonGoogleCloudFunctionCompletePlatforms,
PythonGoogleCloudFunctionHandlerField,
PythonGoogleCloudFunctionRuntime,
ResolvedPythonGoogleHandler,
ResolvePythonGoogleHandlerRequest,
)
from pants.backend.google_cloud_function.python.target_types import rules as target_type_rules
from pants.backend.python.target_types import (
PexCompletePlatformsField,
PythonRequirementTarget,
PythonSourcesGeneratorTarget,
)
from pants.backend.python.target_types import PythonRequirementTarget, PythonSourcesGeneratorTarget
from pants.backend.python.target_types_rules import rules as python_target_types_rules
from pants.build_graph.address import Address
from pants.core.target_types import FileTarget
Expand Down Expand Up @@ -298,17 +295,17 @@ def test_at_least_one_target_platform(rule_runner: RuleRunner) -> None:

runtime = rule_runner.get_target(Address("project", target_name="runtime"))
assert "python37" == runtime[PythonGoogleCloudFunctionRuntime].value
assert runtime[PexCompletePlatformsField].value is None
assert runtime[PythonGoogleCloudFunctionCompletePlatforms].value is None

complete_platforms = rule_runner.get_target(
Address("project", target_name="complete_platforms")
)
assert complete_platforms[PythonGoogleCloudFunctionRuntime].value is None
assert (":python37",) == complete_platforms[PexCompletePlatformsField].value
assert (":python37",) == complete_platforms[PythonGoogleCloudFunctionCompletePlatforms].value

both = rule_runner.get_target(Address("project", target_name="both"))
assert "python37" == both[PythonGoogleCloudFunctionRuntime].value
assert (":python37",) == both[PexCompletePlatformsField].value
assert (":python37",) == both[PythonGoogleCloudFunctionCompletePlatforms].value

with pytest.raises(
ExecutionError,
Expand Down
15 changes: 12 additions & 3 deletions src/python/pants/backend/python/util_rules/pex.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,13 @@ def generate_pex_arg_list(self) -> Iterator[str]:


@rule
async def digest_complete_platforms(
complete_platforms: PexCompletePlatformsField,
async def digest_complete_platform_addresses(
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 threw up my hands with rule graph errors, unions and the like. This refactor works but its a bit clunky in that the subclassing backends each need to add a ~throwaway rule to link in.

Let me know if there is a better way to do all this. The basic chain is:

  1. I want to subclass a field to change its doc (PexCompletePlatformsField -> {PythonAwsLambdaCompletePlatforms, GoogleCloudFunctionCompletePlatforms})
  2. There is a pre-existing rule which turns a PexCompletePlatformsField into another thing and the engine does not allow subclasses; so ...

addresses: UnparsedAddressInputs,
) -> CompletePlatforms:
original_file_targets = await Get(
Targets,
UnparsedAddressInputs,
complete_platforms.to_unparsed_address_inputs(),
addresses,
)
original_files_sources = await MultiGet(
Get(
Expand All @@ -124,6 +124,15 @@ async def digest_complete_platforms(
return CompletePlatforms.from_snapshot(snapshot)


@rule
async def digest_complete_platforms(
complete_platforms: PexCompletePlatformsField,
) -> CompletePlatforms:
return await Get(
CompletePlatforms, UnparsedAddressInputs, complete_platforms.to_unparsed_address_inputs()
)


@frozen_after_init
@dataclass(unsafe_hash=True)
class PexRequest(EngineAwareParameter):
Expand Down