-
Notifications
You must be signed in to change notification settings - Fork 87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ensure exception is available when BackgroundConsumer open stream fails #357
Changes from 6 commits
1a64fff
97c2363
2b24714
947b657
ad1f29f
7ec7832
c7f63bd
3304ef1
e120a0c
753a591
3c1daa9
9423063
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -276,7 +276,11 @@ def open(self): | |||||||||||||||||||||||||||||||
request_generator = _RequestQueueGenerator( | ||||||||||||||||||||||||||||||||
self._request_queue, initial_request=self._initial_request | ||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||
call = self._start_rpc(iter(request_generator), metadata=self._rpc_metadata) | ||||||||||||||||||||||||||||||||
try: | ||||||||||||||||||||||||||||||||
call = self._start_rpc(iter(request_generator), metadata=self._rpc_metadata) | ||||||||||||||||||||||||||||||||
except exceptions.GoogleAPICallError as exc: | ||||||||||||||||||||||||||||||||
self._on_call_done(exc) | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. on_call_done expects a future (based on the name of its argument). How does it work with an exception here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The exception will be raised in the call back python-api-core/google/api_core/bidi.py Lines 608 to 610 in 8f73d2e
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The change that I proposed is exactly the same behaviour as the python-api-core/google/api_core/bidi.py Lines 441 to 443 in 6251eab
From grpc/grpc#10885 (comment), I added this note in c7f63bd There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I reverted c7f63bd because python-api-core/google/api_core/grpc_helpers.py Lines 174 to 182 in 6251eab
The behaviour proposed in this PR is the same as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed the code in 753a591 to raise |
||||||||||||||||||||||||||||||||
raise | ||||||||||||||||||||||||||||||||
parthea marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
request_generator.call = call | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In examining this file, it seems to me that
_RequestQueueGenerator._is_active
(lines 91 et seq) should bedef
d asreturn self.call is not None and self.call.is_active()
. The way it's currently written it will returnTrue
whenself.call == None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in e120a0c