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

Add support for .NET 8 and Native AOT for ASP.NET Core bridge libraries. #1668

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

normj
Copy link
Member

@normj normj commented Feb 11, 2024

Description of changes:
Update the ASP.NET Core bridge library to target .NET 8 and support Native AOT. For Amazon.Lambda.AspNetCoreServer I did a major version bump because I dropped support for .NET Core 3.1. .NET Core 3.1 has been out of support for quite sometime and Lambda has blocked function updates since May 2023. By dropping .NET Core 3.1 that simplified the logic especially using the trimming attributes which aren't available in .NET Core 3.1. I didn't major version Amazon.Lambda.AspNetCoreServer.Hosting because it never targeted .NET Core 3.1 and the changes done in Amazon.Lambda.AspNerCoreServer won't affect hosting.

The main trick for getting Native AOT working via Amazon.Lambda.AspNetCoreServer.Hosting was forcing the user to pass in the source generator serializer with a new overload of the AddAWSLambdaHosting. The existing one defaults to the reflection based source generator so it was marked as RequiresUnreferencedCode. To avoid duplicating logic I pushed a lot of the logic of AddAWSLambdaHosting into a utility method leaving the AddAWSLambdaHosting overrides to handle configuring the serializer. So the code got moved all around but it is mostly just to put in a separate method to avoid code duplication.

Along the way I did some clean up of test projects for example the RuntimeSupport unit tests were still targeting .NET Core 2.1 and updated them to .NET 8.

Testing

I have made sure all of the tests continue to work and I have build a ASP.NET Core app with these library for Native AOT and confirmed I got no trim warnings and the function worked correctly.

Some unit test cases were deleted because they tested deprecated behavior that has been removed. Specifically the IWebHostBuilder which isn't used anymore.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

/// registration for Lambda.
/// </summary>
/// <returns></returns>
[Obsolete("Functions should migrate to CreateHostBuilder and use IHostBuilder to setup their ASP.NET Core application. In a future major version update of this library support for IWebHostBuilder will be removed for non .NET Core 2.1 Lambda functions.")]
Copy link
Member Author

Choose a reason for hiding this comment

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

This obsolete method was deleted because it was here to support .NET Core 2.1. The message even said we would delete in the future. The future is now.

@@ -395,16 +393,20 @@ private async Task<ExecutionInfo> ExecHandlerAsync(string handler, string dataIn
Client = testRuntimeApiClient
};

var loggerAction = actionWriter.ToLoggingAction();
Copy link
Member Author

Choose a reason for hiding this comment

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

This was moved down because now that the tests are targeting .NET 8 instead of .NET Core 2.1 it uses the newer log level logger. The problem with that is the new LambdaBootstrap(handlerWrapper, initializer.InitializeAsync) done a little bit above this overrides the test hack done by UserCodeLoader.SetCustomerLoggerLogAction to redirect the logging. I put this after new LambdaBootstrap(handlerWrapper, initializer.InitializeAsync) to override custom logger action after bootstrap does its work.

@normj normj merged commit e9b92eb into dev Feb 15, 2024
3 checks passed
@normj normj deleted the normj/aspnet-net8 branch February 15, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants