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

Clarify runtime vs. complete_platforms for serverless. #18001

Merged

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented Jan 14, 2023

A common pitfall is 1st using runtime, encountering issues, migrating
to complete_platforms and still running into issues due to not
deleting runtime. Hopefully this helps clarify you generally want to
use these two fields as XOR.

A common pitfall is 1st using `runtime`, encountering issues, migrating
to `complete_platforms` and still running into issue due to not deleting
`runtime`. Hopefully this helps clarify you generally want to use these
two fields as XOR.
@@ -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 ...

@@ -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.

Copy link
Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

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

I can't offhand see a more succinct way to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants