Port AWS Lambda (V1 and V2) to use the Target API #9491
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # No Rust changes made. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] # No JVM changes made.
# Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # No Rust changes made. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] # No JVM changes made.
# Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # No Rust changes made. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] # No JVM changes made.
Prework for generalizing Configurations. # Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # No Rust changes made. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] # No JVM changes made.
Eric-Arellano
commented
Apr 8, 2020
Comment on lines
+7
to
+10
| class PythonAwsLambdaBinaryField(StringField): | ||
| """Target spec of the `python_binary` that contains the handler.""" | ||
|
|
||
| alias = "binary" |
There was a problem hiding this comment.
I think this should be required, but Benjy and I couldn't figure out how the V1 code works because it also looks at the dependencies field to determine what the binary is..So, I left it as optional.
NB that this binding is solely so that we parse the BUILD file and can pass it down to V1. V1 will still express its "requiredness" itself.
Comment on lines
-37
to
+43
| files_content = list( | ||
| self.request_single_product(FilesContent, Params(created_awslambda.digest)) | ||
| ) | ||
| files_content = self.request_single_product(FilesContent, created_awslambda.digest) |
There was a problem hiding this comment.
This change is possible because we made Collections much more powerful recently, e.g. implementing __len__ on it.
| @@ -113,18 +113,18 @@ class Binary(Goal): | |||
| registered_target_types: RegisteredTargetTypes, | |||
| ) -> CreatedBinary: | |||
| target = wrapped_target.target | |||
| binary_config_types: Iterable[Type[BinaryConfiguration]] = union_membership.union_rules[ | |||
| config_types: Iterable[Type[BinaryConfiguration]] = union_membership.union_rules[ | |||
There was a problem hiding this comment.
Name change to make this Configuration code a little more generic. I'm starting to look for patterns that make sense to be factored up.
# Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # No Rust changes made. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] # No JVM changes made.
# Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # No Rust changes made. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] # No JVM changes made.
# Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # No Rust changes made. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] # No JVM changes made.
* a handler entry point is not a normal entry point, i.e. it does not work with things like `./v2 binary` * there's no reason to only allow one single `sources` file # Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # No Rust changes made. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] # No JVM changes made.
# Delete this line to force CI to run Clippy and the Rust tests. [ci skip-rust-tests] # No Rust changes made. # Delete this line to force CI to run the JVM tests. [ci skip-jvm-tests] # No JVM changes made.
benjyw
approved these changes
Apr 8, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
V1 vs. V2 implementations
The implementations are different. V1 expects you to pass the field
binaryto specify a specificpython_binary, whereas V2 is much more flexible in allowing you to specifysourcesand/ordependenciesso that thepython_awslambdatarget acts like apython_binarytarget. This results in much less boilerplate.V1:
V2:
In a followup, we will deprecate V1 AWS Lambda for the V2 implementation.
Result: better error message on invalid target types
Note that this will only error if no targets were valid. It will not error if some targets are invalid and some are valid.
--
This also fixes bad
sourcefield entries in the root level BUILD.