-
Notifications
You must be signed in to change notification settings - Fork 19
DOCSP-48153: Add sync vs. async content to migration page #241
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
Conversation
✅ Deploy Preview for docs-pymongo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
||
* - ``find_one_and_replace()`` | ||
- .. code-block:: python | ||
Asynchronous Methods |
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.
Note to tech reviewer – I cut this section down to just a link because I don't know if it makes sense to have two different sources of truth for async methods.
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! A few questions and suggestions.
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 with some tiny suggestions!
source/reference/migration.txt
Outdated
{+driver-async+} driver. This is due to the {+driver-async+} driver using the native | ||
``asyncio`` library instead of thread-based executors. | ||
|
||
Users experiencing performance slowdown should consider using synchronous {+driver-short+} |
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.
We should call out that users should first identify if they actually need to be using asynchronous Python here, or if their use case is better served by the synchronous API.
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.
Can take a stab at this in the next revision.
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.
Tiny wording change suggestions for clarity, otherwise looks good!
source/reference/migration.txt
Outdated
@@ -113,10 +113,15 @@ The following Motor method signatures behave differently in the {+driver-async+} | |||
|
|||
Motor users may experience a degradation of performance when switching to the | |||
{+driver-async+} driver. This is due to the {+driver-async+} driver using the native |
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.
{+driver-async+} driver. This is due to the {+driver-async+} driver using the native | |
{+driver-async+} driver. This is due to the {+driver-async+} driver using native |
source/reference/migration.txt
Outdated
|
||
Users experiencing performance slowdown should consider using synchronous {+driver-short+} | ||
with ``async.loop.run_in_executor()`` for asynchronous compatibility. To learn more, see | ||
``asyncio`` library instead of thread-based executors. Thread-based executors |
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.
``asyncio`` library instead of thread-based executors. Thread-based executors | |
``asyncio`` tasks instead of thread-based executors. Thread-based executors |
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-48153
Staging Links
Self-Review Checklist