Make rpc calls using an asyncio task context MOD-3632

[freider]

Wrap all grpc api calls in asyncio.tasks that are cancelled when the Client is closed, and raise a custom error when that happens + for future calls on an already closed Client.

This allows the client to shut down more gracefully by stopping rpc calls prior to unwinding the synchronicity event loop, instead of triggering a generic StreamTerminatedError or similar that we don't know wether to retry or not.

[erikbern]

Instead of translating the grpc stubs dynamically, we might want to generate our own stub code using a plugin. It's pretty sparsely documented but should be doable: https://medium.com/cloud-native-daily/create-and-run-a-protobuf-plugin-9471e16e8ad

Might be a bit cleaner in terms of tracebacks etc, and could let us do some other things too (i.e. enable retries and handle grpc exceptions more cleanly)

[freider]

Instead of translating the grpc stubs dynamically, we might want to generate our own stub code using a plugin. It's pretty sparsely documented but should be doable: https://medium.com/cloud-native-daily/create-and-run-a-protobuf-plugin-9471e16e8ad

Might be a bit cleaner in terms of tracebacks etc, and could let us do some other things too (i.e. enable retries and handle grpc exceptions more cleanly)

Do you mean like a fork of grpclib's stub generator (https://github.com/vmagamedov/grpclib/blob/master/grpclib/plugin/main.py) or more like an additional generated stub that references the original? Feels like it would be nice to avoid forking/patching, so we can take advantage of potential improvements in grpclib itself, so maybe the latter?

[erikbern]

Do you mean like a fork of grpclib's stub generator (https://github.com/vmagamedov/grpclib/blob/master/grpclib/plugin/main.py) or more like an additional generated stub that references the original? Feels like it would be nice to avoid forking/patching, so we can take advantage of potential improvements in grpclib itself, so maybe the latter?

yeah no forking right? just an extra plugin

[freider]

Do you mean like a fork of grpclib's stub generator (https://github.com/vmagamedov/grpclib/blob/master/grpclib/plugin/main.py) or more like an additional generated stub that references the original? Feels like it would be nice to avoid forking/patching, so we can take advantage of potential improvements in grpclib itself, so maybe the latter?

yeah no forking right? just an extra plugin

Yeah exactly - that's what I ended up with, just wanted to clarify that it matched your idea :)

[freider]

This appears to not be used anymore 🕺

[freider]

Moving these to the modal.client module since they are now tied to a client instance

[freider]

@prbot approve

[modal-pr-review-automation]

Approved 👍. @mwaskom will follow-up review this.