[async-await] Code generation for "simple, but safe" wrapper client calls. #1261

simonjbeaumont · 2021-09-11T20:10:17Z

This PR adds codegen support for the "simple, but safe wrappers" that were part of the proposal for async/await support, added in #1231.

Codegen has also been re-run for the example Echo service.

glbrntt · 2021-09-13T09:24:31Z

Sources/Examples/Echo/Model/echo.grpc.swift

+      callOptions: callOptions
+    )
+    return try await withTaskCancellationHandler {
+      try Task.checkCancellation()


What's the right amount of checking for cancellation? This feels like a lot.

Well I guess these are all legitimate early exit points. We could omit as many of these as we like, each omission resulting in some different semantics in the presence of cancellation.

We might want to enumerate the things we are OK with happening after a cancellation?

Sources/Examples/Echo/Model/echo.grpc.swift

Sources/GRPC/AsyncAwaitSupport/GRPCClient+AsyncAwaitSupport.swift

simonjbeaumont · 2021-09-13T15:43:42Z

@glbrntt I rebased now that you merged the other PR.

glbrntt

This broadly looks good. I left a couple of comments inline. Could we also have some very simple tests to exercise these? (You can bootstrap these: https://github.com/grpc/grpc-swift/blob/async-await/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift)

Sources/Examples/Echo/Model/echo.grpc.swift

Sources/GRPC/AsyncAwaitSupport/GRPCClient+AsyncAwaitSupport.swift

Sources/Examples/Echo/Model/echo.grpc.swift

Sources/protoc-gen-grpc-swift/Generator-Client+AsyncAwait.swift

simonjbeaumont · 2021-09-16T10:35:00Z

@glbrntt I see a rebase will be necessary since the merge of #1264. Would you like time to review before that?

Sources/protoc-gen-grpc-swift/Generator-Client+AsyncAwait.swift

Signed-off-by: Si Beaumont <beaumont@apple.com>

glbrntt

Cool -- this looks great modulo one nit. There's also an outstanding comment from a previous review:

Could we also have some very simple tests to exercise these? (You can bootstrap these: https://github.com/grpc/grpc-swift/blob/async-await/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift)

Signed-off-by: Si Beaumont <beaumont@apple.com>

simonjbeaumont · 2021-09-16T14:19:05Z

@glbrntt wrote:

Cool -- this looks great modulo one nit. There's also an outstanding comment from a previous review:

Could we also have some very simple tests to exercise these? (You can bootstrap these: https://github.com/grpc/grpc-swift/blob/async-await/Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift)

Oops, missed this one. Done in 57998ff.

glbrntt · 2021-09-16T14:21:48Z

Tests/GRPCTests/AsyncAwaitSupport/AsyncIntegrationTests.swift

@@ -73,6 +73,13 @@ final class AsyncIntegrationTests: GRPCTestCase {
    }
  }

+  func testUnaryWrapper() {
+    XCTAsyncTest {
+      let response = try await self.echo.get(.with { $0.text = "hello" })


The 'wrapped' API is so much nicer!

glbrntt

Awesome! Thank you Si!

…alls. (grpc#1261) This PR adds codegen support for the "simple, but safe wrappers" that were part of the proposal for async/await support, added in grpc#1231. Codegen has also been re-run for the example Echo service.

…alls. (#1261) This PR adds codegen support for the "simple, but safe wrappers" that were part of the proposal for async/await support, added in #1231. Codegen has also been re-run for the example Echo service.

glbrntt requested changes Sep 13, 2021

View reviewed changes

simonjbeaumont commented Sep 13, 2021

View reviewed changes

Sources/GRPC/AsyncAwaitSupport/GRPCClient+AsyncAwaitSupport.swift Show resolved Hide resolved

simonjbeaumont requested a review from glbrntt September 13, 2021 15:39

simonjbeaumont force-pushed the sb-async-await-wrapper-gen branch from f6b1293 to d53a9d6 Compare September 13, 2021 15:44

glbrntt requested changes Sep 13, 2021

View reviewed changes

simonjbeaumont requested a review from glbrntt September 13, 2021 19:26

glbrntt reviewed Sep 15, 2021

View reviewed changes

simonjbeaumont requested a review from glbrntt September 16, 2021 10:10

glbrntt reviewed Sep 16, 2021

View reviewed changes

Sources/protoc-gen-grpc-swift/Generator-Client+AsyncAwait.swift Outdated Show resolved Hide resolved

simonjbeaumont added 16 commits September 16, 2021 11:53

codegen: Simple, but safe, async-await client wrappers

6aa960b

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: swiftformat

8a81bca

Signed-off-by: Si Beaumont <beaumont@apple.com>

Add wrapper extensions to API

77b1d86

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Use factored out wrappers in codegen

11237d5

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: swiftformat

f1be3ea

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Stop sending requests once we have a response

85981da

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Make all the GRPCChannel async-await extensions internal

ac0a2d9

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Add extensions for Sequence

f41bb9b

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Add defaulted type params to extensions

a6d5c49

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Rename internal perform(_:_:) to perform(_:with:)

9d84e17

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Style: move return up a line

44f4668

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Codegen for Sequence conforming RequestStream

c18d521

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Add note about the use of AsyncStream.init(wrapping:)

04af228

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: use Types.responseStream(of: self.methodOutputName)

4ed24dd

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Refactor codegen to remove duplication

e6c5872

Signed-off-by: Si Beaumont <beaumont@apple.com>

fixup: Following rebase

2d1e988

Signed-off-by: Si Beaumont <beaumont@apple.com>

simonjbeaumont force-pushed the sb-async-await-wrapper-gen branch from a62c2e9 to 2d1e988 Compare September 16, 2021 10:56

glbrntt reviewed Sep 16, 2021

View reviewed changes

fixup: Stupid newlines

eaf234b

Signed-off-by: Si Beaumont <beaumont@apple.com>

test: Add tests for wrappers to integration tests

57998ff

Signed-off-by: Si Beaumont <beaumont@apple.com>

glbrntt reviewed Sep 16, 2021

View reviewed changes

glbrntt approved these changes Sep 16, 2021

View reviewed changes

glbrntt added the 🆕 semver/minor Adds new public API. label Sep 16, 2021

glbrntt merged commit c693ebc into grpc:async-await Sep 16, 2021

[async-await] Code generation for "simple, but safe" wrapper client calls. #1261

[async-await] Code generation for "simple, but safe" wrapper client calls. #1261

Uh oh!

Conversation

simonjbeaumont commented Sep 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glbrntt Sep 13, 2021

Choose a reason for hiding this comment

Uh oh!

simonjbeaumont Sep 13, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonjbeaumont commented Sep 13, 2021

Uh oh!

glbrntt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonjbeaumont commented Sep 16, 2021

Uh oh!

Uh oh!

glbrntt left a comment

Choose a reason for hiding this comment

Uh oh!

simonjbeaumont commented Sep 16, 2021

Uh oh!

glbrntt Sep 16, 2021

Choose a reason for hiding this comment

Uh oh!

glbrntt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

simonjbeaumont commented Sep 11, 2021 •

edited

Loading