
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2018-07-29 09:25 byalex.gronholm, last changed2022-04-11 14:59 byadmin. This issue is nowclosed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 8547 | merged | python-dev,2018-07-29 11:41 | |
| PR 8717 | merged | alex.gronholm,2018-08-09 16:19 | |
| PR 8759 | merged | benjamin.peterson,2018-08-14 04:15 | |
| Messages (21) | |||
|---|---|---|---|
| msg322620 -(view) | Author: Alex Grönholm (alex.gronholm)* | Date: 2018-07-29 09:25 | |
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 | |||
| msg323302 -(view) | Author: Yury Selivanov (yselivanov)*![]() | Date: 2018-08-08 21:06 | |
New changesetcca4eec3c0a67cbfeaf09182ea6c097a94891ff6 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 | |||
| msg323303 -(view) | Author: Yury Selivanov (yselivanov)*![]() | Date: 2018-08-08 21:08 | |
Thank you for the contribution! | |||
| msg323324 -(view) | Author: Eric Snow (eric.snow)*![]() | Date: 2018-08-09 14:41 | |
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() inModules/_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. :/ | |||
| msg323325 -(view) | Author: Yury Selivanov (yselivanov)*![]() | Date: 2018-08-09 14:46 | |
> 2. change the check to PyUnicode_CheckExact()I'd be OK with this, but why is this important? | |||
| msg323326 -(view) | Author: Yury Selivanov (yselivanov)*![]() | Date: 2018-08-09 14:47 | |
As a side note, Alex, what do you think about appending coroutine's name to Task's name if the latter is autogenerated? | |||
| msg323327 -(view) | Author: Alex Grönholm (alex.gronholm)* | Date: 2018-08-09 14:51 | |
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. | |||
| msg323328 -(view) | Author: Alex Grönholm (alex.gronholm)* | Date: 2018-08-09 14:52 | |
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. | |||
| msg323329 -(view) | Author: Yury Selivanov (yselivanov)*![]() | Date: 2018-08-09 14:55 | |
> 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 aFalseSo Eric is right, there's a small discrepancy between Python and C version. | |||
| msg323330 -(view) | Author: Alex Grönholm (alex.gronholm)* | Date: 2018-08-09 15:00 | |
Ok, I understand. But is the conversion a bad thing then? | |||
| msg323334 -(view) | Author: Yury Selivanov (yselivanov)*![]() | Date: 2018-08-09 15:16 | |
> 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. | |||
| msg323337 -(view) | Author: Alex Grönholm (alex.gronholm)* | Date: 2018-08-09 15:26 | |
> 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? | |||
| msg323338 -(view) | Author: Yury Selivanov (yselivanov)*![]() | Date: 2018-08-09 15:34 | |
Let's just reuse this issue, it's just a small fix. | |||
| msg323339 -(view) | Author: Alex Grönholm (alex.gronholm)* | Date: 2018-08-09 15:36 | |
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. | |||
| msg323340 -(view) | Author: Eric Snow (eric.snow)*![]() | Date: 2018-08-09 15:39 | |
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(). :) | |||
| msg323341 -(view) | Author: Alex Grönholm (alex.gronholm)* | Date: 2018-08-09 15:44 | |
> 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 = nameelse: self._name = str(name)I don't know about you, but it looks pretty awkward to me. | |||
| msg323342 -(view) | Author: Yury Selivanov (yselivanov)*![]() | Date: 2018-08-09 16:00 | |
Please just change PyUnicode_Check to PyUnicode_CheckExact in C Task.__init__ and use the same if check in C Task.set_name. | |||
| msg323343 -(view) | Author: Alex Grönholm (alex.gronholm)* | Date: 2018-08-09 16:06 | |
> 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. | |||
| msg323344 -(view) | Author: Yury Selivanov (yselivanov)*![]() | Date: 2018-08-09 16:09 | |
> 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? | |||
| msg323345 -(view) | Author: Alex Grönholm (alex.gronholm)* | Date: 2018-08-09 16:10 | |
> 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. | |||
| msg323497 -(view) | Author: Benjamin Peterson (benjamin.peterson)*![]() | Date: 2018-08-14 04:32 | |
New changesetaa4e4a40db531f7095513a4b0aa6510f18162a07 by Benjamin Peterson in branch 'master':Make regular expressions in test_tasks.py raw strings. (GH-8759)https://github.com/python/cpython/commit/aa4e4a40db531f7095513a4b0aa6510f18162a07 | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:04 | admin | set | github: 78451 |
| 2018-08-14 04:32:34 | benjamin.peterson | set | nosy: +benjamin.peterson messages: +msg323497 |
| 2018-08-14 04:15:47 | benjamin.peterson | set | pull_requests: +pull_request8235 |
| 2018-08-09 20:49:59 | yselivanov | set | resolution: fixed |
| 2018-08-09 16:19:30 | alex.gronholm | set | pull_requests: +pull_request8202 |
| 2018-08-09 16:10:11 | alex.gronholm | set | messages: +msg323345 |
| 2018-08-09 16:09:39 | yselivanov | set | messages: +msg323344 |
| 2018-08-09 16:06:05 | alex.gronholm | set | messages: +msg323343 |
| 2018-08-09 16:00:39 | yselivanov | set | messages: +msg323342 |
| 2018-08-09 15:44:29 | alex.gronholm | set | resolution: fixed -> (no value) messages: +msg323341 |
| 2018-08-09 15:39:36 | eric.snow | set | resolution: fixed messages: +msg323340 |
| 2018-08-09 15:36:11 | alex.gronholm | set | messages: +msg323339 |
| 2018-08-09 15:34:27 | yselivanov | set | resolution: fixed -> (no value) messages: +msg323338 |
| 2018-08-09 15:26:42 | alex.gronholm | set | messages: +msg323337 |
| 2018-08-09 15:16:55 | yselivanov | set | messages: +msg323334 |
| 2018-08-09 15:00:20 | alex.gronholm | set | messages: +msg323330 |
| 2018-08-09 14:55:28 | yselivanov | set | messages: +msg323329 |
| 2018-08-09 14:52:23 | alex.gronholm | set | messages: +msg323328 |
| 2018-08-09 14:51:25 | alex.gronholm | set | messages: +msg323327 |
| 2018-08-09 14:47:49 | yselivanov | set | messages: +msg323326 |
| 2018-08-09 14:46:46 | yselivanov | set | messages: +msg323325 |
| 2018-08-09 14:41:24 | eric.snow | set | nosy: +eric.snow messages: +msg323324 |
| 2018-08-08 21:08:30 | yselivanov | set | status: open -> closed resolution: fixed messages: +msg323303 stage: patch review -> resolved |
| 2018-08-08 21:06:54 | yselivanov | set | messages: +msg323302 |
| 2018-07-29 11:41:30 | python-dev | set | keywords: +patch stage: patch review pull_requests: +pull_request8063 |
| 2018-07-29 09:25:37 | alex.gronholm | create | |