chore: avoid checking instance on each stream call#529
Conversation
|
Converting to draft as presubmits are failing |
product-auto-label
Bot
added
size: m
|
@parthea I fixed the tests. I'm prioritizing some of these gapic performance improvements, so I'm fine if you'd rather close this for now and revisit it later. But this one should be fairly straight-forward |
parthea
left a comment
parthea
left a comment
There was a problem hiding this comment.
LGTM, but please wait for @vchudnov-g to review
|
@vchudnov-g Please could you review? |
vchudnov-g
left a comment
vchudnov-g
left a comment
There was a problem hiding this comment.
Thanks for doing this!
|
|
||
| @pytest.mark.asyncio | ||
| async def test_wrap_stream_errors_type_error(): | ||
| async def test_wrap_errors_type_error(): |
There was a problem hiding this comment.
It would be helpful to comment that the explicit type of error you're checking for is the "Unexpected type of callable" (which now happens at wrapping time). As per my other comment, if that were a specialized subclass of TypeError, we could check for that and the code would be self-documenting, so we wouldn't need an extra comment.
| return _wrap_stream_errors(callable_, _WrappedStreamStreamCall) | ||
| else: | ||
| return _wrap_stream_errors(callable_) | ||
| raise TypeError("Unexpected type of callable: {}".format(type(callable_))) |
There was a problem hiding this comment.
nit (not a blocker, consider for the future): consider in cases like this raising a custom subclass of TypeError, so that we can check the exception type in tests
3 participants
_wrap_stream_errors currently tests the type of the call on each call. This PR moves the logic to happen once, at method wrap time.