Drop sync and closure APIs #222
Conversation
fabianfett
force-pushed
the
ff-drop-sync-and-closure-api
branch
from
f16b170
to
19b03ed
Compare
| logger: Logger, | ||
| eventLoop: EventLoop, | ||
| allocator: ByteBufferAllocator) { | ||
| public init(requestID: String, |
There was a problem hiding this comment.
@tomerd We probably need to discuss this one! Maybe we want to add a __forTest_Only() helper method instead of the public initializer.
| public static func test<In: Decodable>( | ||
| _ closure: @escaping Lambda.CodableVoidClosure<In>, | ||
| with event: In, | ||
| public static func test<Handler: LambdaHandler>( |
There was a problem hiding this comment.
@tomerd Lambda.test only supports new async/await lambdas:
XCTAssertNoThrow(result = try Lambda.test(MyLambda.self, with: input))The benefit is that the Lambda goes through the complete lifecycle with this approach. Initializer and handler are called.
| @@ -36,7 +36,7 @@ extension Lambda { | |||
| /// `ByteBufferAllocator` to allocate `ByteBuffer` | |||
| public let allocator: ByteBufferAllocator | |||
|
|
|||
| internal init(logger: Logger, eventLoop: EventLoop, allocator: ByteBufferAllocator) { | |||
| public init(logger: Logger, eventLoop: EventLoop, allocator: ByteBufferAllocator) { | |||
There was a problem hiding this comment.
@tomerd We probably need to discuss this one! Maybe we want to add a __forTest_Only() helper method instead of the public initializer.
There was a problem hiding this comment.
yes this not ideal. why is it needed?
| // XCTAssertEqual(result, "echo" + input) | ||
| // } | ||
|
|
||
| #if DEBUG |
| @@ -35,7 +35,7 @@ struct Handler: EventLoopLambdaHandler { | |||
| } | |||
| } | |||
|
|
|||
| Lambda.run(Handler()) | |||
| Lambda.run { $0.eventLoop.makeSucceededFuture(Handler()) } | |||
There was a problem hiding this comment.
this is kind of meh, we can potentially also support an API that returns an instance and then wraps in successful future to avoid pushing this to the user side
There was a problem hiding this comment.
Will readd with a separate PR.
| @@ -78,27 +54,19 @@ public enum Lambda { | |||
| return String(cString: value) | |||
| } | |||
|
|
|||
| #if swift(>=5.5) | |||
There was a problem hiding this comment.
should we just require 5.5? feels wrong to drop support for closures and support < 5.5
There was a problem hiding this comment.
We talked about this last time. We wanted to stay with 5.2 to allow existing frameworks (Vapor, Noze, Hummingbird, Smoke, and friends) to integrate with Lambda without needing to bump their requirements. For integration into those frameworks developers will nearly always use the Lambda.Lifecycle APIs.
Normal Lambda users should require 5.5 and just use the async APIs.
I guess with this we will do what most of the Swift Server ecosystem is going todo... Make async/await the normal, leave escape hatch for advanced use cases using futures.
Fixes #212.
Motivation:
For
1.0we want to have an API that we are happy with going forward. Everything we are not 100% happy with should go out before1.0. With the release of Swift 5.5, we don't want to have callback based APIs anymore. For this reason we drop support for callback based APIs with this PR.This also includes the way we deal with Lambda lifecycle: We drop the API's in which a user could create a Handler themselves and pass it to Lambda. The Handler creation is now always a part of the Lambda's lifecycle. For advanced users (users who wish to implement bridges to server frameworks) the advanced Lambda.Lifecycle API will stay.
Modifications:
Result:
Smaller, cleaner API for 1.0.