fix: close GRPC channel when we dispose of clients #779
Conversation
googlebot
added
the
cla: yes
schmidt-sebastian
force-pushed
the
mrschmidt/shutdown
branch
from
0d2f97e
to
31609e9
Compare
schmidt-sebastian
changed the title
|
FYI @thebrianchen |
| @@ -401,7 +401,8 @@ export class Firestore { | |||
|
|
|||
| logger('Firestore', null, 'Initialized Firestore GAPIC Client'); | |||
| return client; | |||
| } | |||
| }, | |||
| /* clientDestructor= */ (client: GapicClient) => client.close() | |||
There was a problem hiding this comment.
hmmm - do we really have .close()? :)
There was a problem hiding this comment.
Not yet - but this PR shows how we could add it. We still have to add it to the Gapic generator though (if you are okay with the general direction).
There was a problem hiding this comment.
@alexander-fenster Do you know what the next sets of steps would be if we wanted to add this support?
We need to also include the field_definition and client protos
|
@alexander-fenster PR rebased against the Typescript branch and updated. |
Codecov Report
@@ Coverage Diff @@
## master #779 +/- ##
==========================================
- Coverage 90.36% 90.35% -0.02%
==========================================
Files 25 25
Lines 2814 2820 +6
Branches 707 707
==========================================
+ Hits 2543 2548 +5
- Misses 118 119 +1
Partials 153 153
Continue to review full report at Codecov.
|
…estore into mrschmidt/shutdown
schmidt-sebastian
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
See #768, especially #768 (comment)
This is a "Request for comments" PR if this is a viable strategy to dispose of unused GRPC clients, in the hope that this will mitigate memory leaks. If feasible, some of the code in this PR needs to be pushed upstream to the Gapic generator.