Add gRPC aio stub and servicer generation

[RobinMcCorkell]

This requires shabbyrobe/grpc-stubs#33

The idea here is to break the problem down into two parts:

1. Modify the Servicer type to accommodate both sync and async implementations
2. Support an async Stub type

I also didn't try to be 100% type safe with this, since the upstream GRPC code makes that very hard/impossible within the constraints of the Python type system. Instead, I aimed for good-enough, where most errors can be caught, but it won't stop you from e.g. using an async Servicer method in a sync Server.

Servicer type

Here, we need a base Servicer type that can be subtyped by both sync implementations and async implementations. Return types are easy: use a Union which allows either Awaitable return or direct sync return.

Streaming request parameters are more difficult: I used a bit of hackery to allow subtyping by both Iterator and AsyncIterator, by defining a _MaybeAsyncIterator base class that inherits from both Iterator and AsyncIterator, and ignores the resulting method definition clash errors. Since _MaybeAsyncIterator is never actually used in implementations, this seems alright.

Stub type

I tried all sorts of stuff here:

• A MultiCallable with @overload on __call__ to support both async and sync usage: doesn't work due to Invalid type inferred when overloading __call__ with generic self-type python/mypy#8283
• @overload on the stub itself, to pick the async or sync MultiCallable: @overload only works on functions declared with def, and even crazy stuff involving @property or __getattr__ didn't work.

In the end, I settled on defining a new type (that doesn't exist at runtime), AsyncStub, that directly implements the async stub type. Users need to manually cast to this type, but after that initial creation it does work as you'd expect.

This would be improved by adding a FooAsyncStub = FooStub into the upstream GRPC generation, since then runtime will match the type stubs, and we could even define an __init__ to avoid the casting requirement.

[nipunn1313]

This looks great!

Checking my understanding. IIUC, at runtime, grpc actually supports both based on what is passed into the __init__ of the stub. However, because of the overload issue that you mentioned, it is difficult to type both of the Stub type together in one type. Is that right?

Another possible (bandaid) approach could be to add a flag for sync vs async stubs. More accurately, a flag to determine which one works by default. Would make it easier to work with async-only codebases.
https://github.com/nipunn1313/mypy-protobuf/blob/main/mypy_protobuf/main.py#L1024

If the above linked mypy/overload issue gets fixed, it would be relatively easy to migrate from a flagged approach -> a universally working approach.

Would appreciate a couple bits of housekeeping

• Updating the README with some instructions on how to use
• Update CHANGELOG under upcoming with a description
• Update the contributors section of readme w/ your name (if you are ok with it, I like to keep a list crediting everyone)

And regarding the tests - I gotta figure out why the tests aren't passing. Looks like something broke about the github actions setup around 3 months ago. Give me some time to look into that.

This fixes a longtime issue #216 !

[nipunn1313]

#490 - should help with making CI healthy again (requires rebase)

[nipunn1313]

Check out #217 - which was another attempt at this. There was a lot of discussion there about the various ways to handle the exact scenario you're running into.

I think the approach you took is good, but might suggest one small improvement, which is to add a flag which flips which one (sync/async) is the default behavior.

Ideally we can oneday migrate to something with overloads eventually and remove this wart - but ok with adding it today to unblock you.

[RobinMcCorkell]

I hesitate to add flags, given the comment #217 (comment) on that PR:

There's still a couple of shortcomings to the flag approach

• Flags are hard to maintain - can blow up test matrix.
• Flag must be issued on a per-invocation basis rather than on a per-service basis. Meaning you can't have multiple services within a file with different implementation types. There is at least a workaround in this case (split into two files and make multiple invocations to protoc)

To keep the scope of this PR small, I suggest we avoid flags and maintain default = sync, which is backwards compatible. Changing the default behaviour won't be useful for me anyway (I work in a monorepo where different users want the sync or async versions, and I don't control the mypy protobuf execution).

[nipunn1313]

cool - looks just about good to me - If you are able get the tests and lint to pass, it should be good to go. Thanks for contributing!

[RobinMcCorkell]

@nipunn1313 it took a few modifications to your CI (please confirm I did the right thing) but it's now passing

[nipunn1313]

thanks for the improvements to run_test.sh

I'll be the first to admit that the script is a bit fragile, but thanks for powering through.

[nipunn1313]

this should already be happening in the pyproject.toml

Was intentionally having the generated .pyi files match black formatting. Would be willing to relax that constraint if it's really difficult to maintain, but I think it's worth a little bit of effort to see if we can get it to work because people end up command-clicking to pyi files in VSCode.

Doesn't need to block this diff - I can look later before releasing - filed #489 .

[nipunn1313]

I'll put some time in to try and do a new release at some point in the next couple weeks. Bear with me!

[artificial-aidan]

@nipunn1313 any update on a release? Hoping to get this rolled into some refactoring we're doing. Testing it from latest right now. Seems to be working

[nipunn1313]

Thanks for the bump. Has fallen off my plate. Let me check it out again the weekend. Appreciate the patience!

[BrendanGraham14]

@nipunn1313 Any update on a release with this change included? This will be super helpful!

[nipunn1313]

Just did it today! Had to do a bit of work to upgrade dependencies and fiddle with the linters and get everything working, but it worked.

[jyggen]

@RobinMcCorkell python/mypy#8283 seems to be fixed now (and available in MyPy 1.7). Is MultiCallable with @overload on __call__ worth another shot? Tried to take a stab at it myself, but wasn't entirely sure how to do it.

[RobinMcCorkell]

@RobinMcCorkell python/mypy#8283 seems to be fixed now (and available in MyPy 1.7). Is MultiCallable with @overload on __call__ worth another shot? Tried to take a stab at it myself, but wasn't entirely sure how to do it.

I would suggest waiting for a year or so to allow mypy 1.7 to make it into the wild. I know for certain my company will be on an older version for a while.