[issue25304] Add run_coroutine_threadsafe() to asyncio

[R. David Murray]

R. David Murray added the comment:

I think the 'versionchanged' should say "3.4.4, 3.5.1".  We've already had one 
user confused by the fact that it isn't in 3.5.0.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Yury Selivanov]

Yury Selivanov added the comment:

Good catch!  Fixed in https://hg.python.org/cpython/rev/b34c42e46e7b

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Vincent Michel]

Vincent Michel added the comment:

I attached a patch that should sum up all the points we discussed.

I replaced the `call_soon_threadsafe` example with:
  loop.call_soon_threadsafe(callback, *args)
cause I couldn't find a simple specific usage. Let me know if you think of a 
better example.

--
Added file: http://bugs.python.org/file40691/run_coroutine_threadsafe_2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

Guido van Rossum added the comment:

loop *is* required for this function. It's just that there's an earlier, 
general comment describing it as optional for all functions in this section.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Vincent Michel]

Vincent Michel added the comment:

> The docs look good.

Should I add a note to explain why the loop argument has to be explicitly 
passed? (there is a note at the beginning of the `task functions` section 
stating "In the functions below, the optional loop argument ...")

> What do you need to add to the concurrency and multithreading section?

This section provides an example to schedule a coroutine from a different 
thread using `ensure_future` and `call_soon_threadsafe`. This example should be 
replaced with another usage of `call_soon_threadsafe` and another paragraph 
about `run_coroutine_threadsafe` should be added.

> I agree on the try/except

Do you think the exception should be re-raised for the logger?

> can you add that to the same diff? 

All right, should I make another PR on the asyncio github repo as well?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Yury Selivanov]

Yury Selivanov added the comment:

>> Should I add a note to explain why the loop argument has to be explicitly
passed? (there is a note at the beginning of the `task functions` section
stating "In the functions below, the optional loop argument ...")

> Oh, that's a good idea. It's sort of clear because it starts with "from a
different thread" but it doesn't hurt to say the same thing in different
ways, because it's an easy mistake to make.

Can we make 'loop' a required argument (no default None value)?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Vincent Michel]

Vincent Michel added the comment:

I attached the first version of the documentation for 
`run_coroutine_threadsafe`. The `Concurrency and multithreading` section also 
needs to be updated but I could already use some feedback.

Also, I think we should add a `try-except` in the callback function, especially 
since users can set their own task factory. For instance:
  
loop.set_task_factory(lambda loop, coro: i_raise_an_exception)

will cause the future returned by `run_coroutine_threadsafe` to wait forever. 
Instead, we could have:

except Exception as exc:
if future.set_running_or_notify_cancel():
future.set_exception(exc)

inside the callback to notify the future.

--
keywords: +patch
Added file: http://bugs.python.org/file40689/run_coroutine_threadsafe_doc.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

Guido van Rossum added the comment:

> Should I add a note to explain why the loop argument has to be explicitly
passed? (there is a note at the beginning of the `task functions` section
stating "In the functions below, the optional loop argument ...")

Oh, that's a good idea. It's sort of clear because it starts with "from a
different thread" but it doesn't hurt to say the same thing in different
ways, because it's an easy mistake to make.

>> What do you need to add to the concurrency and multithreading section?

> This section provides an example to schedule a coroutine from a different
thread using `ensure_future` and `call_soon_threadsafe`. This example
should be replaced with another usage of `call_soon_threadsafe` and another
paragraph about `run_coroutine_threadsafe` should be added.

Oh, yes, definitely update that!

> > I agree on the try/except
>
> Do you think the exception should be re-raised for the logger?
>

Yes.

> > can you add that to the same diff?
>
> All right, should I make another PR on the asyncio github repo as well?
>

No, it's easier for me to copy the changes from CPython into the asyncio
repo.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

Guido van Rossum added the comment:

The docs look good. (I assume you've generated the HTML and checked that the 
output looks good, links are clickable etc.) What do you need to add to the 
concurrency and multithreading section?

I agree on the try/except; can you add that to the same diff? (Merging diffs 
into three different branches is a pain so I want to have to do as little as 
possible of it.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[R. David Murray]

R. David Murray added the comment:

I thought you might be interested to know what the name suggests to a relative 
newcomer to asyncio.

When I saw this issue opened, I though "oh, good, an easy way to submit a 
coroutine from a thread in my test code, now I don't have to write 
'loop.call_soon_threadsafe(loop.create_task, my_coroutine(loop=loop))".  Which 
I suspect you are going to tell me is stupid code, but the point is that that's 
what the name 'call_coroutine_threadsafe' made me think it was going to do.  
After reading the linked issue I was very confused about what it was actually 
going to do, as I've never gotten around to looking up what ensure_future does.

If I'm understanding correctly now, I think my assumption was pretty much 
correct, and the only difference is that I get back something I can actually 
manipulate if I need to, making it even better, and which the documentation 
should make clear when I get to read it :)

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Vincent Michel]

Vincent Michel added the comment:

While I was working on the documentation update, I realized that what we called 
`run_coroutine_threadsafe` is actually a thread-safe version of 
`ensure_future`. What about renaming it to `ensure_future_threadsafe`? It might 
be a bit late since `run_coroutine_threadsafe` has been committed, but I think 
it is worth to be considered. I can see two benefits:

- it is less confusing, because it has the same name and using the same 
prototype as `ensure_future`
- it accepts futures and awaitables

The docstring would be "Wrap a coroutine, an awaitable or a future in a 
concurrent.futures.Future.". The documentation would explain that it works like 
`ensure_future` except:
1. its execution is threadsafe
2. it provides a thread-safe future instead of a regular future

I attached an implementation of it. Also, note that I added a `try-except` in 
the callback, which is not mandatory but probably a good thing have.

In any case, I'll keep working on the documentation update.

--
Added file: http://bugs.python.org/file40672/ensure_future_threadsafe.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

Guido van Rossum added the comment:

I'm against that idea. I don't really see a great important future for this 
method either way: It's just a little bit of glue between the threaded and 
asyncio worlds, and people will learn how to use it by finding an example.

The existing ensure_future() function is mostly meant as an internal helper, 
with the understanding that libraries written on top of asyncio might also need 
this functionality. Basically I want people writing asyncio code not to have to 
worry about the difference between futures and coroutines, because they can 
both be awaited for; ensure_future() helps preserve that illusion for code that 
really needs a future (usually to add a callback).

But honestly I *don't* want to encourage flipping back and forth between 
threads and event loops; I see it as a necessary evil. The name we currently 
have is fine from the POV of someone coding in the threaded world who wants to 
hand off something to the asyncio world.

Why would someone in the threaded world have an asyncio.future that they need 
to wait for? That sounds like they're mixing up the two worlds -- or they 
should be writing asyncio code instead of threaded code.

OTOH, I would love to see the documentation update!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Yury Selivanov]

Yury Selivanov added the comment:

> - it is less confusing, because it has the same name and using the same 
> prototype as `ensure_future`
> - it accepts futures and awaitables

I like this idea.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Yury Selivanov]

Yury Selivanov added the comment:

> But honestly I *don't* want to encourage flipping back and forth between 
> threads and event loops; I see it as a necessary evil. The name we currently 
> have is fine from the POV of someone coding in the threaded world who wants 
> to hand off something to the asyncio world.

I agree, but on the other hand supporting awaitables in addition to coroutines 
would be great. Can we at least add that?

--
nosy: +Yury.Selivanov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

Guido van Rossum added the comment:

@rdm: thanks, you nailed it. :-)

@yury: but where would you have gotten the awaitable in the first place? It's 
easy to see how to get a coroutine -- just define it (with either @coroutine or 
async def) and call it -- and I think that's the only use case that matters 
here.

I'm unclear how you could have gotten another awaitable (e.g. an asyncio.Future 
instance) in another thread -- except by an utterly confused user.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Yury Selivanov]

Yury Selivanov added the comment:

> @yury: but where would you have gotten the awaitable in the first place? It's 
> easy to see how to get a coroutine -- just define it (with either @coroutine 
> or async def) and call it -- and I think that's the only use case that 
> matters here.

Just a few days ago Andrew Svetlov raised a question on github -- how could he 
refactor `aiohttp.get()` coroutine to be a coroutine (compatible with coroutine 
ABC) *and* an async context manager.  With my recent commit, `ensure_future()` 
now accepts not just coroutines or futures, but all objects with `__await__`.

So if you have a library with an API exposed through coroutines, it would be 
great if you have some freedom do refactor it and keep it compatible with 
asyncio functions such as `run_coroutine_threadsafe()`

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

Guido van Rossum added the comment:

Well, I still worry that this is just going to encourage more people to try and 
call awaitables from threaded code, and through some circuitous path of 
misunderstandings (probably involving StackOverflow :-) end up using this 
function.

(Pretty much the worst-case scenario would be calling 
run_coroutine_threadsafe() targeting the *current* loop, and then blocking the 
thread waiting for its result. Deadlock!)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

Changes by Guido van Rossum :


--
nosy: +vxgmichel

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

Changes by Guido van Rossum :


--
stage:  -> commit review
type:  -> behavior

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

Guido van Rossum added the comment:

It's done. But I am hoping you (or someone) will add docs.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Roundup Robot]

Roundup Robot added the comment:

New changeset 25e05b3e1869 by Guido van Rossum in branch '3.4':
Issue #25304: Add asyncio.run_coroutine_threadsafe(). By Vincent Michel.
https://hg.python.org/cpython/rev/25e05b3e1869

New changeset e0db10d8c95e by Guido van Rossum in branch '3.5':
Issue #25304: Add asyncio.run_coroutine_threadsafe(). By Vincent Michel. (Merge 
3.4->3.5.)
https://hg.python.org/cpython/rev/e0db10d8c95e

New changeset 69829a7fccde by Guido van Rossum in branch 'default':
Issue #25304: Add asyncio.run_coroutine_threadsafe(). By Vincent Michel. (Merge 
3.5->3.6.)
https://hg.python.org/cpython/rev/69829a7fccde

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25304] Add run_coroutine_threadsafe() to asyncio

[Guido van Rossum]

New submission from Guido van Rossum:

This is a placeholder bug to reference the PR: 
https://github.com/python/asyncio/pull/273 by Vincent Michel.

--
assignee: gvanrossum
components: asyncio
messages: 252215
nosy: gvanrossum, haypo, yselivanov
priority: normal
severity: normal
status: open
title: Add run_coroutine_threadsafe() to asyncio
versions: Python 3.4, Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com