Lambda Worker Sample#152
Conversation
| @@ -0,0 +1,299 @@ | |||
| # Lambda Worker | |||
There was a problem hiding this comment.
No best place to put this comment... I think the structure of this sample can be improved. As it is, looking at the source, it just looks like a pile of commingled files. Take a look at https://github.com/temporalio/samples-dotnet/tree/main/src/Encryption as an example of clean separation. Maybe the following structure:
Deploy(contains all of the scripts)Starter(same as the current Starter directory)Worker(all of the *.csproj and *.cs files for the worker, rename project file to have.Workeron end, remember to update deploy-lambda.sh with that change too, the compile exclusion in the csproj is removable)README.mdremains at the root*.samplefiles remain at the root, but maybe rename to*.template.yamlif they are to be copy/pasted with substitutions.
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "TemporalioSamples.EnvConfig", "src\EnvConfig\TemporalioSamples.EnvConfig.csproj", "{52CE80AF-09C3-4209-8A21-6CFFAA3B2B01}" | ||
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "TemporalioSamples.LambdaWorker", "src\LambdaWorker\TemporalioSamples.LambdaWorker.csproj", "{D1D6B7AD-0E12-4D8C-83BE-9CE3EB916DD7}" |
There was a problem hiding this comment.
These two projects should be grouped into a folder in this solution file, probably called LambdaWorker.
| } | ||
|
|
||
| [Fact] | ||
| public void ConfigureWorkerOptions_UsesEnvironmentOverrides() |
There was a problem hiding this comment.
I don't think this test is actually testing anything about the sample itself. Feels like it's testing the product or even just the EnvironmentVariableScope behavior, which isn't used anywhere else other than this test) rather than demonstrating any correctness about the sample.
| @@ -0,0 +1,52 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Should this file have a .sh extension? Maybe also rename to enable-telemetry.sh, or something more indicative of what this file does?
| --function-name "$FUNCTION_NAME" \ | ||
| --timeout 60 \ | ||
| --layers "$ADOT_LAYER_ARN" \ | ||
| --environment "Variables={TEMPORAL_API_KEY=${TEMPORAL_API_KEY},SSL_CERT_FILE=/etc/pki/tls/certs/ca-bundle.crt,OPENTELEMETRY_COLLECTOR_CONFIG_URI=/var/task/otel-collector-config.yaml,TEMPORAL_TASK_QUEUE=${TASK_QUEUE},TEMPORAL_LAMBDA_WORKFLOW_ID_PREFIX=${WORKFLOW_ID_PREFIX},TEMPORAL_LAMBDA_DEPLOYMENT_NAME=${DEPLOYMENT_NAME},TEMPORAL_LAMBDA_BUILD_ID=${BUILD_ID}}" \ |
There was a problem hiding this comment.
I don't think that TEMPORAL_LAMBDA_WORKFLOW_ID_PREFIX is needed here since only the starter project consumes it.
|
|
||
| ```bash | ||
| TEMPORAL_CONFIG_FILE="$PWD/temporal.toml" \ | ||
| mise exec dotnet@8 -- dotnet run \ |
There was a problem hiding this comment.
Why invoke via mise here and not for the dotnet publish call in the deploy-lambda.sh script? I think remove mise since it's not a prereq and doesn't add value here.
| @@ -0,0 +1,25 @@ | |||
| // @@@SNIPSTART dotnet-lambda-worker | |||
There was a problem hiding this comment.
Why snipsync here? I thought we were applying docs snipsync in temporalio/features? Which allows us to evolve the samples without breaking docs.
| } | ||
| } | ||
| } No newline at end of file | ||
| } |
There was a problem hiding this comment.
nit: undo unrelated change
| } | ||
|
|
||
| [Fact] | ||
| public void ConfigureWorkerOptions_SetsExpectedWorkerRegistration() |
There was a problem hiding this comment.
This also feels like testing SDK behavior rather than sample behavior. I would drop it.
|
|
||
| ## Files | ||
|
|
||
| | File | Description | |
There was a problem hiding this comment.
This can be drastically simplified with the directory refactor mentioned above. I don't know if you really need to call out each individual file either. Maybe with the refactor, just state what each directory contains in general.
4 participants
Needs temporalio/sdk-dotnet#721
What was changed
Add sample code for a worker running on AWS Lambda
Why?
SDK-level implementation of serverless workers on AWS Lambda
Checklist
Closes
How was this tested:
Deployed a Lambda worker and ran a workflow