Skip to content
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

Swift: Add docs for SyncSession.reconnect() #3062

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

dacharyc
Copy link
Collaborator

@dacharyc dacharyc commented Oct 30, 2023

Pull Request Info

A few questions for a tech reviewer based on the similar docs in Kotlin:

  • In Kotlin, we say "The SDK also automatically calls this method when a device toggles off airplane mode" - is this accurate in the Swift SDK, also? I didn't see anything special for this in the SDK PR so wondering if this is something in Core?
  • In Kotlin, we say "the SDK forces all sync sessions to attempt to reconnect immediately" - is this also true in Swift, or is it forcing only the SyncSession you call it on to reconnect? Does this change with session multiplexing enabled/disabled?
  • Kotlin has a callout about not being able to force a reconnection within the socket read timeout duration - is this also true in Swift?
  • It seems the RLMSyncSession got docs for the new method, but Swift's SyncSession didn't. Could we add this, or am I looking in the wrong place?

Jira

Staged Changes

The rest of the examples in the test file were still using Partition-Based Sync, so I also updated those examples to FS. Those examples are at:

Reminder Checklist

If your PR modifies the docs, you might need to also update some corresponding
pages. Check if completed or N/A.

  • Create Jira ticket for corresponding docs-app-services update(s), if any
  • Checked/updated Admin API
  • Checked/updated CLI reference

Review Guidelines

REVIEWING.md

@github-actions
Copy link

github-actions bot commented Oct 30, 2023

Readability for Commit Hash: 3e6814b

You can see any previous Readability scores (if they exist) by looking
at the comment's history.

Readability scores for changed documents:

  • source/sdk/swift/sync/sync-session: Grade Level: 11.0, Reading Ease: 40.65

For Grade Level, aim for 8 or below.

For Reading Ease scores, aim for 60 or above:

Score Difficulty
90-100 Very Easy
80-89 Easy
70-79 Fairly Easy
60-69 Medium
50-59 Fairly Hard
30-49 Hard
0-29 Very Hard

For help improving readability, try Hemingway App.

Copy link
Collaborator

@cbullinger cbullinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy updates LGTM, though I'm curious if .reconnect() does apply to all sessions.
And FYI - the last example on the Configure & Open a Synced Realm page still uses PBS in the snippet: /examples/generated/code/start/Authenticate.snippet.offline-login.swift

offline and attempts to reconnect using an incremental backoff strategy.

In Swift SDK version 10.44.0 and later, you can choose to manually trigger a
reconnect attempt with ``SyncSession.reconnect()``
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to link to the API ref here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swift doesn't have an API ref for this method, it seems - only Objective-C. I meant to ask about this in the tech review - I'll add a note to my PR description about it 👍

@dacharyc
Copy link
Collaborator Author

And FYI - the last example on the Configure & Open a Synced Realm page still uses PBS in the snippet: /examples/generated/code/start/Authenticate.snippet.offline-login.swift

Yeah, that comes from a different test file - the Authenticate file - which I wasn't updating here. I was just updating the other examples in the file I was touching. At some point I should do a bigger sweep to update all the PBS examples in Swift.

@tgoyne
Copy link
Collaborator

tgoyne commented Oct 31, 2023

reconnect() will skip the reconnect delay for all sessions in the same App regardless of if multiplexing is enabled. I think saying that it'll force a reconnect might be misleading, as it won't cause a reconnection if we got a fatal error. It also doesn't do anything if the connection isn't currently waiting to reconnect, i.e. if it's already connected or is in the process of trying to connect.

On Apple platforms core listens for network change notifications and automatically triggers a reconnection immediately after receiving one. Some SDKs may implement similar logic for other platforms.

A longstanding problem we've had with jazzy is that part of our Swift API is just re-exporting obj-c types, and jazzy doesn't include those things in the generated Swift docs. Since we're no longer working on jazzy, this is unlikely to ever get fixed.

@dacharyc dacharyc merged commit a0e50ac into mongodb:master Nov 2, 2023
5 of 6 checks passed
@dacharyc dacharyc deleted the DOCSP-33926 branch November 2, 2023 20:42
@docs-builder-bot
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants