AWS Lambda#721
Conversation
| fresh Temporal client and worker, runs until the Lambda deadline minus `ShutdownDeadlineBuffer`, then shuts down and runs | ||
| configured shutdown hooks. | ||
|
|
||
| ## Configuration |
There was a problem hiding this comment.
how do I configure the shutdown timing?
jmaeagle99
left a comment
jmaeagle99
left a comment
There was a problem hiding this comment.
Wanted to provide some feedback; still reviewing.
| Extensions: | ||
|
|
||
| * [Temporalio.Extensions.DiagnosticSource](/api/Temporalio.Extensions.DiagnosticSource.html) | ||
| * [Temporalio.Extensions.Aws.Lambda](/api/Temporalio.Extensions.Aws.Lambda.html) |
| src/Temporalio/bin/Release/*.snupkg | ||
| src/Temporalio.Extensions.DiagnosticSource/bin/Release/*.nupkg | ||
| src/Temporalio.Extensions.DiagnosticSource/bin/Release/*.snupkg | ||
| src/Temporalio.Extensions.Aws.Lambda/bin/Release/*.nupkg |
| "files": [ | ||
| "Temporalio/*.csproj", | ||
| "Temporalio.Extensions.DiagnosticSource/*.csproj", | ||
| "Temporalio.Extensions.Aws.Lambda/*.csproj", |
| }); | ||
| ``` | ||
|
|
||
| #### Client Configuration From Environment |
There was a problem hiding this comment.
Please separate this out from this change and open a separate PR
| <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo"> | ||
| <_Parameter1>Temporalio.Tests</_Parameter1> | ||
| </AssemblyAttribute> | ||
| <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo"> |
There was a problem hiding this comment.
Please alphabetize (by _Parameter1 element)
| /// Gets or sets hooks to run after each invocation's worker has shut down. | ||
| /// </summary> | ||
| #pragma warning disable CA2227 // The public API intentionally allows replacing the list during configuration. | ||
| public IList<Func<CancellationToken, Task>> ShutdownHooks { get; set; } = |
There was a problem hiding this comment.
- I would remove the setter, to avoid accidentally dropping any already-existing hooks when setting a new list (which will now be disallowed). This eliminates the pragma.
- I would actually change the property type to
IReadOnlyCollection<T>and use a private readonlyList<T>backing field; return it from the property getter. Add aAddShutdownHook(Func<T>)method that (1) validates the hook is not null, and (2) adds it to the private backing field. This would eliminate the need for downstream item null checking and prevent resets or mutations of the collection. - And to take it even further, if consumers don't need to inspect the hook list, just make it
internal.
| }; | ||
| config.WorkerOptions.ApplyPostPluginConfiguration(); | ||
|
|
||
| foreach (var hook in config.ShutdownHooks) |
There was a problem hiding this comment.
Becomes unnecessary if https://github.com/temporalio/sdk-dotnet/pull/721/changes#r3438966388 is adopted.
| /// <returns>A Lambda handler delegate.</returns> | ||
| public static Func<object?, ILambdaContext, Task> CreateHandler( | ||
| WorkerDeploymentVersion version, | ||
| Action<LambdaWorkerConfig> configure) => |
There was a problem hiding this comment.
This should be invoked per-invocation as well. I don't think there should be a difference of call behavior just because one is sync and the other is async.
| ApplyDeploymentVersion(options, version); | ||
| ClearConcurrencyLimitsIfTunerSet(options); | ||
| }; | ||
| config.WorkerOptions.ApplyPostPluginConfiguration(); |
There was a problem hiding this comment.
I think forcing the application here is a production smell (because it's doubly called: here and in the worker) and is only necessary for testing without a real worker. I think this should be removed and then have the affected tests simulate the worker behavior.
| /// <remarks> | ||
| /// Don't expose this until there's a use case. | ||
| /// </remarks> | ||
| internal Action<TemporalWorkerOptions>? PostPluginConfiguration { get; set; } |
There was a problem hiding this comment.
Instead of this, could we write our own internal plugin to do it? Would eliminate this new surface area, guarantee last application, show up in worker heartbeat, etc.
3 participants
What was changed
Add helpers for serverless workers running on AWS Lambda
Why?
SDKs require specific helpers to run effectively on AWS Lambda.
Checklist
Closes
How was this tested:
Code in a separate PR in samples-dotnet ( Lambda Worker Sample samples-dotnet#152 ) was used to test deploying and running workflows and activities on a Lambda worker.
Any docs updates needed?