Add ability to provide metadata, timeout & deadline args to requests

[nat-n]

#24

I introduced testing of the generated stubs so that I could test the new functionality.

This is an enhancement of the ServiceStub abstract class that makes it more useful by making it possible to pass all arguments supported by the underlying grpclib request function.

In particular this makes the generated stubs usable with APIs that require clients to provide an authorization token.

It extends to the existing high level API by allowing values to be set on the stub instance, and the low level API by allowing values to be set per call.

[nat-n]

Bump

Any thoughts on this?

This addresses an issue that prevents the generated clients from being usable as they are for some projects.

[boukeversteegh]

Great PR in my opinion!

I have not tested the implementation, but read the changes.

• This PR adds additional constructor parameters to the Client and method parameters to RPC calls. As this is a new feature (that seems quite fundamental), I would approve, as it doesn't break existing code either.
• Nice new test setup, I hope we can use it for some new features and testing other parts of code that are now untested.
• Pipfilelock was added. Not really needed in this PR, but its needed in general. I see you referenced the latest Stable protobuf (3.11.3) and not 3.12rc. That's great. I had so much trouble running pipenv install because the requirement was set to *, which resolved to a preview release on my system, which pipenv refused to install.

[boukeversteegh]

I tried out your branch, and initialized my Service with timeout=3, and can verify that the timeout is indeed respected.

Since the generated clients are still the same (templates.py is not updated), we cannot override the timeout per RPC call. That takes away most of the value of the construction of __resolve_request_kwargs. We could directly read self.timeout etcetera in the two _unary_* methods, and keep the code somewhat concise.

Overriding per RPC call is not so trivial, as it would immediately create a namespace conflict (Input messages containing properties like timeout).

Do you have some plan for supporting per method overrides of these settings in the near future? Otherwise I would remove that supporting code. If you have a suggestion for how to pass the parameters, I'd be interested to make a PR for it.

[nat-n]

@boukeversteegh thanks for the feedback.

On the point of supporting timeout etc in _unary_unary etc methods; you're right that unfortunately there isn't currently a straight forward way to integrate this with the way Stub methods are currently generated, and I'm sure right now of a way to address this. For most cases setting these values at the stub level is probably good enough, though I think it's easily to imagine it being awkward.

However I do think this will need to be addressed at some point because it's also an issue for another fundamental feature i.e. working with stream requests.

But ultimately for now my I still think it's still worthwhile for these lower level methods to support the extra kwargs for the benefit of users who specifically want to set these values per call and don't mind working with a such as "lower level API".

For example:

from my_service import MyServiceStub as  BasicMyServiceStub, MyRequest, MyResponse

class MyServiceStub(BasicMyServiceStub):
    async def try_get_stuff_quickly(self, params: Dict[str, Any], timeout=0.01) -> MyResponse:
        return await self._unary_unary(
            "my_package.MyService/GetStuff", 
            MyRequest(**params),
            MyResponse,
            timeout=timeout,
        )

[boukeversteegh]

You're welcome!
So yeah, I see. Someone could still use those features by subclassing the RPC stub, until we figure out how to pass them per request. I'd say, lets get this merged @danielgtaylor :-)