aws / aws-lambda-dotnet

Libraries, samples and tools to help .NET Core developers develop AWS Lambda functions.
Apache License 2.0
1.58k stars 478 forks source link

Amazon.Lambda.Annotations code generation explicit dependent on Amazon.Lambda.APIGatewayEvents #1204

Open Simonl9l opened 2 years ago

Simonl9l commented 2 years ago

Describe the bug

When experimenting with Amazon.Lambda.Annotations it seem that one also has to have Amazon.Lambda.APIGatewayEvents as a package reference in the project or the generated code can not compile.

This is neither noted in the Amazon.Lambda.Annotations README.md nor logically required if on just wants to deploy an HTTP Triggered Lambda without any API Gateway integration.

Expected Behavior

The generated code to compile without the need to add an explicit dependency, if this is required, that it should be a dependency of Amazon.Lambda.Annotations

Current Behavior

 "/Users/<user>/Projects/<path>/src/TestCGLambda/TestCGLambda.csproj" (default target) (1:7) ->
       (CoreCompile target) -> 
         /Users/<user>/Projects/<path>/TestCGLambda/Function.cs(11,5): error AWSLambda0104: Your project has a missing required package dependency. Please add a reference to the following package: Amazon.Lambda.APIGatewayEvents [/Users/l<user>/Projects/<path>/src/TestCGLambda/TestCGLambda.csproj]
         CSC : error AWSLambda0001: This is a bug. Please run the build with detailed verbosity (dotnet build --verbosity detailed) and file a bug at https://github.com/aws/aws-lambda-dotnet with the build output and stack trace Object reference not set to an instance of an object.   at Amazon.Lambda.Annotations.SourceGenerator.Models.TypeModelBuilder.Build(ITypeSymbol symbol, GeneratorExecutionContext context) [/Users/l<user>/Projects/<path>/src/TestCGLambda/TestCGLambda.csproj]

Reproduction Steps

build sample per README.md omitting Amazon.Lambda.APIGatewayEvents from referenced packages.

Possible Solution

No response

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

Targeted .NET Platform

.NET 6

Operating System and version

OSX Monterey

ashishdhingra commented 2 years ago

@Simonl9l Please refer to the comment https://github.com/aws/aws-lambda-dotnet/issues/1052#issuecomment-1016771760 for the reasoning behind need to add explicit dependency in user project. Let me know if this resolves the confusion.

Simonl9l commented 2 years ago

@ashishdhingra thanks for the pointer...so how do I not need a dependency on that APIGatewayEvents package if I just want a raw HTTP Lambda with no specific binding? I don't now what in the code base has made this a requirement.

ashishdhingra commented 2 years ago

@ashishdhingra thanks for the pointer...so how do I not need a dependency on that APIGatewayEvents package if I just want a raw HTTP Lambda with no specific binding? I don't now what in the code base has made this a requirement.

@Simonl9l Could you please point me to sample which you are referring to in the issue description?

Simonl9l commented 2 years ago

Hi @ashishdhingra

So based on the Amazon.Lambda.Templates::6.2.0, I did a dotnet new lambda.EmptyFunction ... and then copied in the following sample fro the Amazon.Lambda.Annotation README.md into Function.cs:

public class Functions
{
    [LambdaFunction]
    [RestApi("/plus/{x}/{y}")]
    public int Plus(int x, int y)
    {
        return x + y;
    }
}

I noted that the RestAPI attribute syntax needed to be modified to [RestApi(LambdaHttpMethod.Get, "/plus/{x}/{y}")], and also modified that to be HttpApi.

That's it. The intent is not to have any need for the APIGatewayEvents so what do I need to change to eliminate ether dependency? Per the comment you reference It seems that I should not need this dependency not building a Lambda to connect with the API Gateway?

I apologies if Im making rookie mistakes here...and appreciate your continued assistance.

EDIT: Seems commit fc7176398ca01e3e6d7b87f395b674bad57a6e06 added the dependency. This seems entirely unnecessary if one is not binding to APIGatewayEvents, and just providing an HTTP endpoint to say an ALB Target?

Whilst not need in this case, one assumes, in time, this will all be extended to support al the supported bindings?

normj commented 2 years ago

The generated code by the source generator use the types in Amazon.Lambda.APIGatewayEvents when using the RestApi or HttpApi attributes. Your code doesn't need to directly interact with the types Amazon.Lambda.APIGatewayEvents except for right now if you want to customize the response. Since your code is not interacting with Amazon.Lambda.APIGatewayEvents and the package is very small collection of types for serialization is there a reason you are asking to have no dependency on it?

github-actions[bot] commented 2 years ago

This issue has not received a response in 5 days. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

Simonl9l commented 2 years ago

@normj - does the generated code for the [RestApi]' or[HttpApi]both/either/or include references to the types inAmazon.Lambda.APIGatewayEvents`? Since a Lambda can be used in the context of say an ALB Target that has no dependency of the API Gateway?

Does this scenario in fact need its own Attribute such that per the promise clarified by @ashishdhingra per https://github.com/aws/aws-lambda-dotnet/issues/1052#issuecomment-1016771760?

If the argument is to limit package size, what do I need to include the Amazon.Lambda.APIGatewayEvents package is my deployment configuration is not related to the API Gateway.

Simonl9l commented 2 years ago

@normj - Applogies I was being a tad slow before, being new to all this, and not realizing my default assumption are around a prior ALB binding case.

Whist its observed that the ALB/Lambda bindings has limited support with both SAM and the Rider AWS Toolkit, do you have any timelines on when the Amazon.Lambda.ApplicationLoadBalancerEvents will be fully supported with Amazon.Lambda.Annotations vs other bindings?

One assumes there will then be hander support for FromBody ?

normj commented 2 years ago

@Simonl9l I don't have a timeline on when specific event support for ALB will be added. We had been working on other tasks that were taking priority over this library for the past couple months. Now that we are done with those tasks we plan on putting focus back on this library.

Simonl9l commented 2 years ago

@normj great/thanks - please can you let us know where we need to monitor for updates.

Simonl9l commented 2 years ago

@normj - in the mean time per the Event Attributes, specifically:

Event attributes configuring the source generator for the type of event to expect and setup the event source in the CloudFormation template. If an event attribute is not set the parameter to the LambdaFunction must be the event object and the event source must be configured outside of the code.

Please could you help with a shot code sample of how this could be configured outside of the code as ALB Events are natively supported we can then more easily migrate. This may also help others.