Skip to content

Conversation

@natanrolnik
Copy link
Contributor

I've added a bit more comments to the todos-lambda setup. Also fixed some typos.

Ideally, I believe we should aim to update this sample project to use a Function URL instead of API Gateway, as it's simpler to setup in simpler deploys and configurations. Because I didn't want to change the SAM configuration file, I left it as it is.

Copy link
Member

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

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

Thanks for this Natan. I'd completely forgotten to update this example.
One thing I'd like to point out this is structured quite differently from the other examples. I think I'd like to demonstrate how easy it is to swap from server to lambda by structuring this example in a similar way to the server examples.

Can you create an App tagged with @main whose run function calls a buildApplication function in a similar manner to the other examples


// Shut down the AWS client and other services after the lambda
// is done
try await app.shutdown()
Copy link
Member

Choose a reason for hiding this comment

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

This won't get called if lambda.runService() throws an error.
In other places where I've used Soto, I've added an AWSClientService. eg

struct AWSClientService: Service {
    let client: AWSClient

    func run() async throws {
        // Ignore cancellation error
        try? await gracefulShutdown()
        try await self.client.shutdown()
    }
}

Copy link
Member

Choose a reason for hiding this comment

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

Although I think I'm gonna add a PR to SotoCore, which adds a trait to includes ServiceLifecycle and conforms AWSClient to Service so it can be added to the list of services and shutdown is done correctly. So if we hold off merging this we can add that change in here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! Let me know when it's available then.

Copy link
Member

Choose a reason for hiding this comment

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

Just working on it now. It'll need approval from Tim though and he seems quite busy at the moment

Copy link
Member

Choose a reason for hiding this comment

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

Its in 7.9.0

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.

2 participants