[issue34270] Add names to asyncio tasks

[Benjamin Peterson]

Benjamin Peterson  added the comment:


New changeset aa4e4a40db531f7095513a4b0aa6510f18162a07 by Benjamin Peterson in 
branch 'master':
Make regular expressions in test_tasks.py raw strings. (GH-8759)
https://github.com/python/cpython/commit/aa4e4a40db531f7095513a4b0aa6510f18162a07


--
nosy: +benjamin.peterson

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Benjamin Peterson]

Change by Benjamin Peterson :


--
pull_requests: +8235

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Change by Yury Selivanov :


--
resolution:  -> fixed

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

Change by Alex Grönholm :


--
pull_requests: +8202

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

Alex Grönholm  added the comment:

> 
I'll do that if you say so, but I'm just saying that the C and Python 
implementations will still remain different in semantics then.

Never mind, that was a brain fart. I keep ignoring the "!" in my mind.

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Yury Selivanov  added the comment:

> I'll do that if you say so, but I'm just saying that the C and Python 
> implementations will still remain different in semantics then.

Probably I'm missing something here. How would they be different?

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Yury Selivanov  added the comment:

Please just change PyUnicode_Check to PyUnicode_CheckExact in C Task.__init__ 
and use the same if check in C Task.set_name.

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

Alex Grönholm  added the comment:

> Please just change PyUnicode_Check to PyUnicode_CheckExact in C Task.__init__ 
> and use the same if check in C Task.set_name.

I'll do that if you say so, but I'm just saying that the C and Python 
implementations will still remain different in semantics then.

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

Alex Grönholm  added the comment:

> On the other had, the matter is made moot by using PyUnicode_CheckExact()

Then, in order to keep the pure Python implementation in sync, we'd have to 
change it to something like this:

if name is None:
   self._name = f'Task-{_task_name_counter()}'
elif isinstance(name, str):
   self._name = name
else:
   self._name = str(name)

I don't know about you, but it looks pretty awkward to me.

--
resolution: fixed -> 

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Eric Snow]

Eric Snow  added the comment:

I'm not too invested in any changes happening at this point, actually. :)  
Mostly I happened to be reading through the commit and noticed the 
inconsistency.  If I had reviewed the PR then I would have asked that it be 
fixed.  So I figured I'd mention it.

FWIW, I don't expect it would cause any problems.  It could result in a 
different (between the two implementations) Task repr if the name's type (a str 
subclass) implements __repr__.  There's also the possibility of side-effects 
(from the implementation of the name's type).  Neither is a big deal 
(especially the latter since it's *not* a common use case).  On the other had, 
the matter is made moot by using PyUnicode_CheckExact(). :)

--
resolution:  -> fixed

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

Alex Grönholm  added the comment:

Which way do we want to change this? Do we want to convert to pure strings or 
retain the original object? In the latter case both the C and Python 
implementations (including set_name()) have to be changed.

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Yury Selivanov  added the comment:

Let's just reuse this issue, it's just a small fix.

--
resolution: fixed -> 

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

Alex Grönholm  added the comment:

> It's not a bad thing, it's just that we don't do it in C Task and we do it in 
> pure Python Task.  Eric wants us to synchronize them so that in a very 
> unlikely scenario where someone uses subclasses of str for names they will 
> have exact same behaviour under both Tasks implementations.

Should a new issue be created for this so I can make a PR against it?

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Yury Selivanov  added the comment:

> Ok, I understand. But is the conversion a bad thing then?

It's not a bad thing, it's just that we don't do it in C Task and we do it in 
pure Python Task.  Eric wants us to synchronize them so that in a very unlikely 
scenario where someone uses subclasses of str for names they will have exact 
same behaviour under both Tasks implementations.

I'd say let's just fix the C version to use PyUnicode_CheckExact.  Even though 
it's highly unlikely somebody ever hits this, there's no reason to keep Python 
and C implementations even slightly out of sync w.r.t. behaviour.

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

Alex Grönholm  added the comment:

Ok, I understand. But is the conversion a bad thing then?

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Yury Selivanov  added the comment:

> I also couldn't figure out yet why PyUnicode_Check() was necessary in the 
> first place. Doesn't PyObject_Str() just increment the refcount if the 
> argument is already a string?

`str()` returns its argument if it's exactly a `builtins.str` instance.  If 
it's a subclass of str, it will construct a `builtins.str` out of it.

>>> class mystr(str):
... pass
>>> a = mystr('aaa')
>>> str(a) is a
False

So Eric is right, there's a small discrepancy between Python and C version.

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

Alex Grönholm  added the comment:

Yury, I have no objections. Furthermore, it would be nice to expose the 
coroutine object publicly, like curio and trio do. It would make life simpler 
for me in some cases.

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

Alex Grönholm  added the comment:

I also couldn't figure out yet why PyUnicode_Check() was necessary in the first 
place. Doesn't PyObject_Str() just increment the refcount if the argument is 
already a string?

Eric, please explain why these changes should be done.

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Yury Selivanov  added the comment:

As a side note, Alex, what do you think about appending coroutine's name to 
Task's name if the latter is autogenerated?

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Yury Selivanov  added the comment:

> 2. change the check to PyUnicode_CheckExact()

I'd be OK with this, but why is this important?

--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Eric Snow]

Eric Snow  added the comment:

FWIW, the C implementation of Task.__init__ is not exactly equivalent to the 
Python implementation (nor to both the C and Python implementation of 
Task.set_name).  In the C impl of Task.__init__ the provided name is used as-is 
if it's an instance of str:

   (_asyncio_Task___init___impl() in Modules/_asynciomodule.c)

   if (name == Py_None) {
   name = PyUnicode_FromFormat("Task-%" PRIu64, ++task_name_counter);
   } else if (!PyUnicode_Check(name)) {
   name = PyObject_Str(name);
   } else {
   Py_INCREF(name);
   }

One of the following should happen, right?

1. fix the Python implementation of Task.__init__() and both impl of 
Task.set_name()
2. change the check to PyUnicode_CheckExact()
3. remove the special-case (i.e. change the C impl to match the Python impl)

p.s. Sorry I did not notice this before it got committed. :/

--
nosy: +eric.snow

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Yury Selivanov  added the comment:

Thank you for the contribution!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Yury Selivanov]

Yury Selivanov  added the comment:


New changeset cca4eec3c0a67cbfeaf09182ea6c097a94891ff6 by Yury Selivanov (Alex 
Grönholm) in branch 'master':
bpo-34270: Make it possible to name asyncio tasks (GH-8547)
https://github.com/python/cpython/commit/cca4eec3c0a67cbfeaf09182ea6c097a94891ff6


--

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Roundup Robot]

Change by Roundup Robot :


--
keywords: +patch
pull_requests: +8063
stage:  -> patch review

___
Python tracker 

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

[issue34270] Add names to asyncio tasks

[Alex Grönholm]

New submission from Alex Grönholm :

Having names on tasks helps tremendously when something goes wrong in a complex 
asyncio application. Threads have names and even trio has the ability to name 
its tasks. This would also greatly benefit PyCharm's concurrency visualization: 
https://www.jetbrains.com/help/pycharm/thread-concurrency-visualization.html#asyncio

--
components: asyncio
messages: 322620
nosy: alex.gronholm, asvetlov, yselivanov
priority: normal
severity: normal
status: open
title: Add names to asyncio tasks
type: enhancement
versions: Python 3.8

___
Python tracker 

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