-
-
Notifications
You must be signed in to change notification settings - Fork 718
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
Expand async docs #2293
Expand async docs #2293
Conversation
If you don't think this is an improvement or necessary, feel free to close, otherwise I'm happy to make any requested changes. |
Errors seem unrelated:
|
See #2298 |
Looks like unrelated TLS errors this time. |
Yes, see #2331
…On Sat, Nov 3, 2018 at 6:36 AM Dave Hirschfeld ***@***.***> wrote:
Looks like unrelated TLS errors this time.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2293 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AASszM-Gfd2THF_P8N0xqz76Y25VD38uks5urXHIgaJpZM4XLqiO>
.
|
docs/source/asynchronous.rst
Outdated
@@ -107,6 +107,23 @@ Python 3 with Tornado | |||
from tornado.ioloop import IOLoop | |||
IOLoop().run_sync(f) | |||
|
|||
If you want to reuse an externally created client you need to run the async | |||
function using the clients loop which can be done with the ``Client.sync`` | |||
function - e.g. |
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.
Statements like reuse "externally created client" and "using the client's loop" seem odd to me. For the first I'm not sure why the client is external or why we're reusing it. I think that this is just a normal client. In the second case I don't think that we can expect all users to understand what an event loop is.
Instead, I recommend something like ...
If you want to use an asyncrhonous function with a synchronous client (one made without the
asynchronous=True
keyword) then you should use theClient.sync
function.
I've also been warned off of using e.g.
in docs. Apparently it's not universally understood by many people for whom English is not their first language.
I think that the example is great.
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.
For me, it was just that I hadn't come across the sync
function before so figured it could do with some more advertising. I'll make the suggested changes...
@mrocklin - if I make the suggested change above would you be happy merging this? If not I guess we can close it... |
Sure. Sorry for the long delay in response. I'm just going through my e-mail backlog now. |
np - I'll try and find the time to updating this PR shortly... |
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 @dhirschfeld!
Closes #2085
I was a little confused how to run an async function using an existing
Client
object. This clarifies the documentation with the extra information provided by @mrocklin in #2085.