-
Notifications
You must be signed in to change notification settings - Fork 61
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for multiplexing and SyncTimeoutOptions #1578
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments ... and when actually verified by CI (currently offline, so PR is not verified even though green).
packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/AppConfiguration.kt
Outdated
Show resolved
Hide resolved
packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/AppConfiguration.kt
Outdated
Show resolved
Hide resolved
packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/sync/SyncTimeoutOptions.kt
Outdated
Show resolved
Hide resolved
packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/sync/SyncTimeoutOptions.kt
Outdated
Show resolved
Hide resolved
...library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/sync/SyncTimeoutOptionsBuilder.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Some minor comments.
...library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/sync/SyncTimeoutOptionsBuilder.kt
Outdated
Show resolved
Hide resolved
packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/AppConfiguration.kt
Outdated
Show resolved
Hide resolved
packages/library-sync/src/commonMain/kotlin/io/realm/kotlin/mongodb/AppConfiguration.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Nabil Hachicha <[email protected]> Co-authored-by: Claus Rørbech <[email protected]> Co-authored-by: clementetb <[email protected]>
…ngodb/AppConfiguration.kt Co-authored-by: Claus Rørbech <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some very minor comments.
Closes #971
This should also put some hooks in place to make it easier to control initial sync performance as reported by the internal help ticket here https://jira.mongodb.org/browse/HELP-47093
Terminology and API docs have been copied from the Swift implementation: realm/realm-swift#8282
This PR will not enable multiplexing by default, this is left for a future release.
It hasn't been possible to write an integration test for this, but I verified the changes manually by inspecting the logs, which at least shows the PING changes:
And the debugger shows the values being set