feat: use GAX retry config for streams #847
Conversation
googlebot
added
the
cla: yes
| @@ -457,21 +459,6 @@ describe('failed transactions', () => { | |||
| ).to.eventually.be.rejectedWith('Final exception'); | |||
| }); | |||
|
|
|||
| it('fails on beginTransaction', () => { | |||
There was a problem hiding this comment.
This test no longer works, since the retries are handled in google-gax, which is replaced in the unit tests with dumb implementation that doesn't retry.
Codecov Report
@@ Coverage Diff @@
## master #847 +/- ##
========================================
- Coverage 88.69% 87.4% -1.3%
========================================
Files 27 27
Lines 16681 16542 -139
Branches 1151 1151
========================================
- Hits 14796 14458 -338
- Misses 1880 2078 +198
- Partials 5 6 +1
Continue to review full report at Codecov.
|
schmidt-sebastian
changed the base branch from
master
to
mrschmidt/removebun
…irestore into mrschmidt/gaxretries
dev/src/index.ts
Outdated
| } | ||
|
|
||
| try { | ||
| const result = await backoff.backoffAndWait().then(func); |
There was a problem hiding this comment.
Mixing await and then in the same line is kind of confusing. Is it the same as the following code?
await backoff.backoffAndWait();
this._lastSuccessfulRequest = new Date().getTime();
return await func();There was a problem hiding this comment.
It's not quite (the successful request should only be set after await func()). I cleaned it up.
dev/src/util.ts
Outdated
| methodName: string, | ||
| config: ClientConfig | ||
| ): boolean { | ||
| const serviceConfig = config.interfaces!['google.firestore.v1.Firestore']!; |
There was a problem hiding this comment.
Just a heads up, this config file will go away soon. By "soon" here I mean half-a-year-ish time. When backend starts supporting returning this configuration via DNS (TXT record I guess), we'll start using it and will stop generating those JSON configs.
There was a problem hiding this comment.
Got it. I will find some clever way to get this configuration in 6 months from now.
Or rather - I will find a clever way to push this code into GAX.
schmidt-sebastian
changed the base branch from
mrschmidt/removebun
to
master
schmidt-sebastian
force-pushed
the
mrschmidt/gaxretries
branch
from
2c30dcc
to
aed7387
Compare
This PR cleans up our request handling a little bit: