Skip to content
This repository has been archived by the owner on May 31, 2021. It is now read-only.

Promote the use of a main coroutine #13

Closed
wants to merge 6 commits into from
Closed

Conversation

vxgmichel
Copy link
Collaborator

@vxgmichel vxgmichel commented Oct 21, 2016

This PR removes all the explicit loop arguments for consistency.

Another solution is to pass the loop explicitly everywhere but this would make the examples more complicated. This approach is sometimes described as a good practice, though I would argue the good practice is to rely on get_event_loop() unless you have a good reason not to (especially for user code).

Partially related: Issue #9


EDIT: This PR has been widely updated to promote the use of a main coroutine.

This practice is re-enforced by asyncio PR #452: Make get_event_loop() return the current loop if called from coroutines/callbacks. This naturally causes most of the loop references to disappear, since all asyncio objects are created in the context of a coroutine. Most of the examples now end up with the following block:

loop = asyncio.get_event_loop()
loop.run_until_complete(main())
loop.close

In python 3.6, it might be replaced with asyncio.run (given PR #465 is merged):

asyncio.run(main())

This PR also fixes the warnings when building the documentation.

@vstinner
Copy link
Contributor

Hum, I would prefer to pass loop explicitly. Later, users can learn how to get ride of the explicit loop parameter. Maybe add a section to explain that?

@asvetlov
Copy link

-1
If you want to encourage writing reliable code -- pass explicit loop
everywhere.

On Fri, Oct 21, 2016, 15:17 Vincent Michel notifications@github.com wrote:

This PR removes all the explicit loop arguments for consistency.

Another solution is to pass the loop explicitly everywhere but this would
make the examples more complicated. This approach is sometimes described as
a good practice, though I would argue the good practice is to embrace
get_event_loop() unless you have a good reason not to (especially for
user code).

Partially related: Issue #9

#9

You can view, comment on, or merge this pull request online at:

#13
Commit Summary

  • Remove all occurrences of "loop=loop"

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#13, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAVwL2IXJPmRbO7007lgo3EXBtzzqKIlks5q2K1hgaJpZM4KdKv4
.

Thanks,
Andrew Svetlov

@vxgmichel
Copy link
Collaborator Author

vxgmichel commented Oct 26, 2016

@asvetlov

If you want to encourage writing reliable code -- pass explicit loop everywhere.

I don't really see how that makes the code more reliable.

get_event_loop() either:

  • returns the event loop for the current context
  • raises an exception if no loop is available

So if the first call to get_event_loop() does not fail, that means the current context has a loop that will be provided to any coroutine or asyncio object requiring it (unless another loop is explicitly specified).

Now the default policy defines "context" as the current thread, meaning get_event_loop() is perfectly safe to use as long as everything happens in the same thread. That should be the case for a large majority of application, since single-threaded concurrency is the whole point behind asyncio. It is also a safe thing to do in a multi-thread environment as long as the policy handles the different contexts correctly.

On the other hand, passing the loop explicitly everywhere might generate some issues, see asyncio issue #390 for instance. On linux, the policy is responsible for providing the child subprocess watcher. So if one chooses to go explicit, the loop should also be attached explicitly:

asyncio.set_event_loop(None)
loop = asyncio.new_event_loop()
if sys.platform != 'win32':
    asyncio.get_child_watcher().attach_loop(loop) 

That is fine to do for testing in depth a library such as aiohttp, but it seems overly complicated for simple user code.

@vstinner
Copy link
Contributor

vstinner commented Oct 26, 2016 via email

@vxgmichel
Copy link
Collaborator Author

@asvetlov
Now that PR #452 has been merged, there are no reasons not to merge the modifications to examples/tcp_echo_client.py. What about the other files? If you don't like to have the loop argument omitted when objects are created outside of the loop, maybe we should promote the use of a main async function?

async def main():
    queue = asyncio.Queue()
    producer_coro = produce(queue, 10)
    consumer_coro = consume(queue)
    await asyncio.gather(producer_coro, consumer_coro))

loop = asyncio.get_event_loop()
loop.run_until_complete(main())
loop.close()

@vstinner
Copy link
Contributor

vstinner commented Nov 7, 2016

I changed my mind, I'm now in favor of omiting the loop parameter.

It seems like even for asyncio "experts" passing explicitly the loop or not is not a simple choice, so I suggest to write a section to explain when the loop parameter can be omitted, and explain the advantage of passing explicitly the loop parameter.

I understand that in most cases, the loop parameter can be omitted, it helps for readability (shorter code). The only advantage is a micro optimization to avoid the cost of the get_event_loop() call?

So maybe explain that the parameter should be omitted, but if you need extreme performance, you can pass it explicitly?

I like methods like loop.create_task() because it avoids the need of a question: the loop is passed explicitly and the code remains simple and obvious :-)

@asvetlov
Copy link

asvetlov commented Nov 7, 2016

Good to me.
Promoting main() coroutine is very important concept.

@vxgmichel vxgmichel force-pushed the remove-explicit-loop branch from e4d2ad5 to 9bed2f6 Compare November 18, 2016 16:01
@vxgmichel vxgmichel changed the title Remove all occurrences of "loop=loop" Promote the use of a main coroutine Nov 18, 2016
@vxgmichel
Copy link
Collaborator Author

This PR has been widely updated to promote the use of a main coroutine.

This practice is re-enforced by asyncio PR #452: Make get_event_loop() return the current loop if called from coroutines/callbacks. This naturally causes most of the loop references to disappear, since all asyncio objects are created within the context of a coroutine. Most of the examples now end up with the following block:

loop = asyncio.get_event_loop()
loop.run_until_complete(main())
loop.close

In python 3.6, it might be replaced with asyncio.run (given PR #465 is merged):

asyncio.run(main())

This PR also fixes the warnings when building the documentation (9bed2f6).

@pya I made separated commits for the webscraper, since this part of the update is pretty heavy:

I tried to make sure the webscraper page still makes sense, but let me know I forgot anything, or if you don't agree with some of the modifications.

@vxgmichel vxgmichel force-pushed the remove-explicit-loop branch from 9bed2f6 to 96d87c5 Compare March 28, 2017 12:56
@vstinner
Copy link
Contributor

I shut down the project: #33

@vstinner vstinner closed this May 31, 2021
@vstinner vstinner deleted the remove-explicit-loop branch May 31, 2021 07:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants