Add endpoint that exposes realm connection state and upload progress #84
Conversation
src/service.ts
Outdated
| @@ -1079,6 +1097,16 @@ export class GraphQLService { | |||
| user, | |||
| }); | |||
|
|
|||
| this.state[path] = <any>{ connection: Realm.Sync.ConnectionState.Connected }; | |||
|
|
|||
| realm.syncSession.addConnectionNotification((state) => { | |||
There was a problem hiding this comment.
Should we remove these listeners when the Realm is closed?
There was a problem hiding this comment.
They're on the session, and that outlives the realm. Besides, when the session is destroyed it lets go of the listeners anyway.
There was a problem hiding this comment.
My concern is that this will add a listener on every request. Perhaps node.js is clever and will recognize that this listener has already been added to the session, but if that's the case, it's not immediately obvious to me. If it's not clever though, I can easily see a long-lived Realm keeping a collection of thousands of listeners doing the same thing.
- Rename state to avoid clashing with IService.state - Change route to be connectionStatus rather than synchronizing - Move getConnection status above the graphql route to make sure it's handled properly - Don't return 404s
|
@fealebenpae @nirinchev Do we think we can complete this for this coming week? I have a meeting with Customer on Wednesday |
|
@nirinchev and I had a brain-storming session today on how to proceed with the leaking listeners issue and we'll try to get this reviewed and merged by tomorrow so we can cut a release for Customer. |
src/service.ts
Outdated
| @@ -1079,6 +1123,26 @@ export class GraphQLService { | |||
| user, | |||
| }); | |||
|
|
|||
| this.connectionStates[path] = { | |||
There was a problem hiding this comment.
We should not override this if it exists.
|
can we merge this to master please? its been sitting since nov29 cc. @geragray |
|
It has some issues, which is why it hasn't been merged. |
No description provided.