search

newrelic / newrelic-lambda-layers

Source code and utilities to build and publish New Relic's public AWS Lambda layers.
https://newrelic.com/products/serverless-aws-lambda
Apache License 2.0
34 stars 42 forks source link

feat: Add initial dotnet Lambda layer support #228

Closed chynesNR closed 1 month ago

chynesNR commented 1 month ago

Similar to the Python Agent, we don't need to build anything. We can grab the necessary files from the download site to publish.

The same Agent binaries support all the possible .NET Lambda runtimes (.NET 6, 7, and 8), so the only layers that need to be created are x64/ARM64.

Next step will be to add tag creation to our Agent release process.

jaffinito commented 1 month ago

Were you able to look at the other agent's tests to see if there is something we could implement for our agent?

chynesNR commented 1 month ago

Were you able to look at the other agent's tests to see if there is something we could implement for our agent?

The tests seem to be mainly testing the wrapper, which we don't have, and the environment variable parsing, which we already have tests for. I'd like to talk about it in pairing tomorrow, though.

chynesNR commented 1 month ago

Were you able to look at the other agent's tests to see if there is something we could implement for our agent?

The tests seem to be mainly testing the wrapper, which we don't have, and the environment variable parsing, which we already have tests for. I'd like to talk about it in pairing tomorrow, though.

After talking it over, I don't think there are any tests that would be appropriate for this repo. We have enough testing in the Agent repo.

fallwith commented 1 month ago

@chynesNR @jaffinito from looking at the recent Ruby PR ...

chynesNR commented 1 month ago

@chynesNR @jaffinito from looking at the recent Ruby PR ...

  • I agree that you don't need tests given that you don't have any .NET code in play in this repo
  • You don't need Dockerfile files given that you're just downloading .NET releases and not compiling anything
  • [TODO] You should update README.md
  • [TODO] For consistency with all other languages, you should consider updating Makefile and have your GHA workflow call make

Thanks for the reminder about the readme. I've updated it to include .NET.

Consistency is a good goal, but I don't think using make would be appropriate for us. All we need to do is call the publish shell script. Python isn't using make, either, so we won't be the only outlier.

chaudharysaket commented 1 month ago

@chynesNR @jaffinito Use of makefile is not recommended. It is recommended to follow the python way. In the future, we will take up task to remove the use of makefile and dockerfile for release of layers - Nodejs, Java, & Ruby. We wanted to do the same, during migration from CircleCI to Github Actions. Due to the required testing time, we went ahead with just the migration of workflow rather than simplifying the release for Nodejs, Java & Ruby.

chynesNR commented 1 month ago

@chynesNR @jaffinito Use of makefile is not recommended. It is recommended to follow the python way. In the future, we will take up task to remove the use of makefile and dockerfile for release of layers - Nodejs, Java, & Ruby. We wanted to do the same, during migration from CircleCI to Github Actions. Due to the required testing time, we went ahead with just the migration of workflow rather than simplifying the release for Nodejs, Java & Ruby.

Thanks, @chaudharysaket -- do you have any other concerns before we merge?

chaudharysaket commented 1 month ago

@chynesNR @jaffinito Use of makefile is not recommended. It is recommended to follow the python way. In the future, we will take up task to remove the use of makefile and dockerfile for release of layers - Nodejs, Java, & Ruby. We wanted to do the same, during migration from CircleCI to Github Actions. Due to the required testing time, we went ahead with just the migration of workflow rather than simplifying the release for Nodejs, Java & Ruby.

Thanks, @chaudharysaket -- do you have any other concerns before we merge?

Hi @chynesNR, While Publishing dotnet test layer, I receive following error - 

An error occurred (ValidationException) when calling the PublishLayerVersion operation: 1 validation error detected: Value '[dotnet6, dotnet7, dotnet8]' at 'compatibleRuntimes' failed to satisfy constraint: Member must satisfy enum value set: [nodejs20.x, provided.al2023, python3.12, java17, nodejs16.x, dotnet8, python3.10, java11, python3.11, dotnet6, java21, nodejs18.x, provided.al2, ruby3.3, java8.al2, ruby3.2, python3.8, python3.9]

dotnet7 is only supported as container image

chynesNR commented 1 month ago

@chynesNR @jaffinito Use of makefile is not recommended. It is recommended to follow the python way. In the future, we will take up task to remove the use of makefile and dockerfile for release of layers - Nodejs, Java, & Ruby. We wanted to do the same, during migration from CircleCI to Github Actions. Due to the required testing time, we went ahead with just the migration of workflow rather than simplifying the release for Nodejs, Java & Ruby.

Thanks, @chaudharysaket -- do you have any other concerns before we merge?

Hi @chynesNR, While Publishing dotnet test layer, I receive following error -

An error occurred (ValidationException) when calling the PublishLayerVersion operation: 1 validation error detected: Value '[dotnet6, dotnet7, dotnet8]' at 'compatibleRuntimes' failed to satisfy constraint: Member must satisfy enum value set: [nodejs20.x, provided.al2023, python3.12, java17, nodejs16.x, dotnet8, python3.10, java11, python3.11, dotnet6, java21, nodejs18.x, provided.al2, ruby3.3, java8.al2, ruby3.2, python3.8, python3.9]

dotnet7 is only supported as container image

Thanks, fixed.