-
Notifications
You must be signed in to change notification settings - Fork 88
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
DOCSP-41778 query size limit #3355
DOCSP-41778 query size limit #3355
Conversation
❌ Deploy Preview for device-sdk failed. Why did it fail? →
|
Readability for Commit Hash: cb7ee5a You can see any previous Readability scores (if they exist) by looking Readability scores for changed documents:
For Grade Level, aim for 8 or below. For Reading Ease scores, aim for 60 or above:
For help improving readability, try Hemingway App. |
9193f1b
to
3ec628f
Compare
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.
Nice work on adding this to the right page for each SDK! But I think we need to refine where we put it on the pages, and also maybe verify some additional details with engineering so we can communicate the limit effectively in the SDK client context.
source/sdk/kotlin/sync/subscribe.txt
Outdated
@@ -178,6 +178,8 @@ This creates an unnamed subscription and adds it to the | |||
:ref:`manually add the subscription <kotlin-sync-add-subscription>` to the | |||
subscription set. | |||
|
|||
.. include:: /includes/warning-device-sync-query-size-limit.rst |
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.
Same issue here as in the Flutter page - the "Subscribe to Queries" and "Manually Manage Subscriptions" sections represent two different APIs, so we would either need to include this warning in both places or move it to the "Flexible Sync RQL Requirements and Limitations" section.
Same thing applies to the remaining pages here - Node, React Native, and Swift.
de73c93
to
50bc2f9
Compare
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.
Thanks for clarifying that detail with engineering! I think we could do a little more tweaking of the wording to remove some unnecessary wordiness and be clearer about the distinction of an individual query size vs. the size of the subscription set.
The **size limit** for a subscription to a query from your client application is **256 kB** | ||
when using Device Sync. Exceeding this limit results in a |
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.
This still reads a little confusing to me. If I have correctly understood that the size limit is per query in the subscription set, not the total subscription set, we might want to reword a little just for clarity:
We don't need to say "when using Device Sync" because we're on the subscription query page where it's understood (hopefully!) that we're talking about Sync subscriptions. And we don't need to say "from your client application" because we're in the SDK client docs.
We should also update the wording on the C++ page once we're happy with this one.
The **size limit** for a subscription to a query from your client application is **256 kB** | |
when using Device Sync. Exceeding this limit results in a | |
The **size limit** for any given query in your subscription set is | |
**256 kB**. Exceeding this limit results in a |
62ff6d0
to
684d115
Compare
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! ✅
5da72dd
to
cb7ee5a
Compare
cb7ee5a
to
8730166
Compare
✨ Staging URL: https://preview-mongodbmongodb.gatsbyjs.io/realm/master/ 🪵 Logs |
Pull Request Info
Jira ticket: https://jira.mongodb.org/browse/DOCSP-41778
Reminder Checklist
Before merging your PR, make sure to check a few things.
Release Notes
C++ SDK
Throughout the SDK Docs: Update the "Flexible Sync RQL Requirements and Limitations" section to include the query size limit when using Device Sync.
Review Guidelines
REVIEWING.md