Firestore: Add 'should_terminate' predicate for clean BiDi shutdown.#8650
Firestore: Add 'should_terminate' predicate for clean BiDi shutdown.#8650tseaver merged 7 commits intogoogleapis:masterfrom tseaver:7826-api_core-firestore-add-should_terminate-to-bidi
Conversation
Sharpen assertions for other '_on_call_done' testcases.
|
I can also add |
|
Is there a reason should_recover couldn't be used for this? Semantically, should not recover and should terminate seem the same? Do we need another argument. Also, note, immediately following this merge, we should ship api_core and rev min versions for firestore and pubsub |
|
@crwilcox See #7826. The goal is to avoid logging an error when the stream is shut down due to "expected" events., typically due to a
|
plamut
left a comment
There was a problem hiding this comment.
The code changes look good to me.
I was able to reproduce the issue as described, but after re-installing api_core and the firestore changes from this PR, I observed a different error:
(venv-3.6) peter@black-box:~/workspace/google-cloud-python/firestore (pr_temp)$ python reproduce-7826.py
on_snapshot()
Thread-ConsumeBidirectionalStream caught unexpected exception 'NoneType' object has no attribute 'target_change' and will exit.
Traceback (most recent call last):
File "/home/peter/workspace/google-cloud-python/firestore/venv-3.6/lib/python3.6/site-packages/google/api_core/bidi.py", line 653, in _thread_main
self._on_response(response)
File "/home/peter/workspace/google-cloud-python/firestore/venv-3.6/lib/python3.6/site-packages/google/cloud/firestore_v1/watch.py", line 429, in on_snapshot
target_change = proto.target_change
AttributeError: 'NoneType' object has no attribute 'target_change'
It might be caused by my local/project setup, but it might as well be unrelated to the error reported in the ticket (which seems to be fixed by this PR).
|
In any case, if it is possible that |
Stream exceptions which pass this predicate will cause clean shutdown of the BiDi thread.
Closes #7826.
Note to reviewers: I recommend looking at this commit-by-commit.