-
Notifications
You must be signed in to change notification settings - Fork 98
fix: ensure exception is available when BackgroundConsumer open stream fails #357
Changes from 2 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) | ||||||||||||||||||||||||||||||||
|
Contributor
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?
Contributor
Author
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
Contributor
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.
Contributor
Author
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
Contributor
Author
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
Contributor
Author
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.
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| 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 bedefd asreturn self.call is not None and self.call.is_active(). The way it's currently written it will returnTruewhenself.call == NoneThere 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