- Notifications
You must be signed in to change notification settings - Fork95
feat: allow disabling response stream pre-fetch#30
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
crwilcox commentedJun 3, 2020
This seems like a reasonable approach. Before merging I would like to take a moment and make sure firestore is 'fixed' with the original change as well, just to make sure this isn't fixing the wrong thing :) |
plamut commentedJun 3, 2020
Sounds good, let me know when you have info on that. :) |
busunkim96 commentedJun 4, 2020
@crwilcox Anything I can do to help this move forward? I accidentally introduced a dependency conflict with a new release of google-api-python-client |
plamut commentedJun 5, 2020
+1, quite a few PubSub users are looking forward to the fix, too. |
space55 commentedJun 9, 2020
Hi! Is there any ETA for having this merged? We're currently blocked on a bugfix for this in our servers. Thanks! |
arithmetic1728 commentedJun 9, 2020
plamut commentedJun 9, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@space55@arithmetic1728 The new PubSub release that includes its part of this fix ison the way. |
space55 commentedJun 9, 2020
Thanks! I'm excited! |
Uh oh!
There was an error while loading.Please reload this page.
Closes#25.
This PR adds the ability to disable automatically pre-fetching the first item of a stream returned by
*-StreamgRPC callables. This hook will be used in PubSub to fix thestalled stream issue, while also not affecting Firestore, since the default behavior is preserved.I realize the fix is far from ideal, but it's the least ugly among the approaches I tried, e.g. somehow passing the flag through
ResumableBidiRpc(it's a messy rabbit hole). On the PubSub side monkeypatching the generated SubscriberClient will be needed, but it's a (relatively) clean one-liner:If/when we merge this, we should also release it, and then we can add
!= 1.17.0to thegoogle-api-coreversion pin in PubSub.PR checklist