[issue6057] sqlite3 error classes should be documented
timm added the comment: I would find it useful to have the exception classes listed in the Python documentation rather than having to refer to two documents, but I don't have strong feelings on this. Given that nobody has fixed this for 4 years, should we just close the ticket? I'd be happy to do the necessary work if this is something we want changed, but I don't want to prepare a patch for a ticket that's never going to go anywhere. -- nosy: +timm ___ Python tracker <http://bugs.python.org/issue6057> ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40894] asyncio.gather() cancelled() always False
New submission from Timm Wagener : It seems like the future subclass returned by asyncio.gather() (_GatheringFuture) can never return True for future.cancelled() even after it's cancel() has been invoked successfully (returning True) and an await on it actually raised a CancelledError. This is in contrast to the behavior of normal Futures and it seems generally to be classified as a minor bug by developers. * Stackoverflow Post: https://stackoverflow.com/questions/61942306/asyncio-gather-task-cancelled-is-false-after-task-cancel * Github snippet: https://gist.github.com/timmwagener/dfed038dc2081c8b5a770e175ba3756b I have created a fix and will create a PR. It seemed rather easy to fix and the asyncio test suite still succeeds. So maybe this is a minor bug, whose fix has no backward-compatibility consequences. However, my understanding is that asyncio.gather() is scheduled for deprecation, so maybe changes in this area are on halt anyways!? # excerpt from snippet async def main(): """Cancel a gather() future and child and return it.""" task_child = ensure_future(sleep(1.0)) future_gather = gather(task_child) future_gather.cancel() try: await future_gather except CancelledError: pass return future_gather, task_child # run future_gather, task_child = run(main()) # log gather state logger.info(future_gather.cancelled()) # False / UNEXPECTED / ASSUMED BUG logger.info(future_gather.done()) # True logger.info(future_gather.exception()) # CancelledError logger.info(future_gather._state) # FINISHED # log child state logger.info(task_child.cancelled()) # True logger.info(task_child.done()) # True # logger.info(task_child.exception()) Raises because _state is CANCELLED logger.info(task_child._state) # CANCELLED -- components: asyncio messages: 370855 nosy: asvetlov, timmwagener, yselivanov priority: normal severity: normal status: open title: asyncio.gather() cancelled() always False type: enhancement versions: Python 3.8 ___ Python tracker <https://bugs.python.org/issue40894> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40894] asyncio.gather() cancelled() always False
Change by Timm Wagener : -- keywords: +patch pull_requests: +19898 stage: -> patch review pull_request: https://github.com/python/cpython/pull/20686 ___ Python tracker <https://bugs.python.org/issue40894> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40894] asyncio.gather() cancelled() always False
Timm Wagener added the comment: Hello Kyle, thanks for reviewing. > I'm starting to agree that it makes sense to override the behavior for > `cancelled()`. However, it might make more sense to replace the > `self._cancel_requested` check with `isinstance(self.exception(), > exceptions.CancelledError)`, as this more accurately indicates if and when > the gather() was cancelled. I don't think we should rely on > `self._cancel_requested` since it would be true before the future is actually > cancelled and not always be correct since the gather() can be cancelled > without setting `self._cancel_requested`. My understanding of the current logic is, that there is a distinction between an explicit `gather().cancel()` and an external `child.cancel()`. This distinction is made by the `_cancel_requested` variable: * **Explicit gather.cancel():** Only an explicit cancellation of the gather task should result in `cancelled() is True`. This explicit cancellation is indicated by `_cancel_requested`. It works regardless of the `return_exceptions` kwarg. value. A `CancelledError` is always raised at the `await` site and `cancelled()` is True, provided that a previous `gather.cancel()` has been `True`. * **External invocation of child.cancel()/Implicit cancellation, finishing of gather:** In this case, no explicit user intent has caused a cancellation of gather. This is reflected by `_cancel_requested` being `False` and `gather.cancelled()` always being `False`. Instead based on the value of the `return_exceptions` kwarg.: * **False:** The normal `set_exception()` is invoked and the child exception is set on the gather future _(in this case a `CancelledError`)_ while also setting state to `FINISHED`. This results in the gather raising at the `await` site and finishing. What makes it slighty confusing though is the exception being a `CancelledError`. However, i believe its otherwise in line with how `Future.cancelled()` behaves for any exception. * **True:** `CancelledErrors` in children are collected and returned amongst other results and exceptions in a list. > Here's one case where relying on `self._cancel_requested` for > future_gather.cancelled() wouldn't work, based on a slight modification of > the reproducer example: As outlined above, i'd assume this falls into the category of _implicit cancellation, finishing of gather without user intent_ for which I'm assuming `cancelled()` should be `False`. However, as mentioned, this is just my assumption based on the logic. One could also take your viewpoint, that `cancelled()` should be `True` when `fut.exception()` is a `CancelledError`. > This should be "was called *as* it finished", not "was called *after* it was > finished". If gather_future.cancel() is called after the future is > `FINISHED`, it will immediately return false since it checks `self.done()` > first I assume that these scenarios apply, when this like `current_task().cancel()` is happening in a `done()` callback or such? It seems like many (corner)-cases can be created and i'm certainly not aware of them. However, as `gather()` adds it's own callback first to its passed futures, and `outer` is not available beforehand for any done callback registration, i'm not sure how much of an effort one would have to make to create the case you described. But maybe I also didn't really understand the point you are getting at. > asyncio.gather() is not deprecated or scheduled for deprecation, it is simply > the loop argument being deprecated. I was actually referring to [this PR comment](https://github.com/python/cpython/pull/6694#issuecomment-543445208) , which I happened to stumble upon while researching for the issue. > We'll likely have TaskGroups in asyncio 3.9 and will simply deprecate > asyncio.gather. That's why I mentioned in the beginning that it may be a small, easy correctness improvement for a Python patch version if it's backwards compatible and not causing too much trouble. But maybe its also not considered that important anymore. I'd leave that up to the reviewers ;) -- ___ Python tracker <https://bugs.python.org/issue40894> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40894] asyncio.gather() cancelled() always False
Timm Wagener added the comment: TLDR; - The intention of the PR is to make a future from gather return that cancelled() is True if, and only if, cancel() has successfully been called on it (explicit user intent) and it was awaited/has finished. All other finishing is not considered explicit user intent and has the state FINISHED (with exception or result), even if the exception is a CancelledError. As mentioned, just my interpretation based on the logic i've seen. -- ___ Python tracker <https://bugs.python.org/issue40894> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40894] asyncio.gather() cancelled() always False
Timm Wagener added the comment: Ok, seems reasonable as well. I'll change it as proposed. -- ___ Python tracker <https://bugs.python.org/issue40894> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue40894] asyncio.gather() cancelled() always False
Timm Wagener added the comment: Proposed changes are done and test coverage is extended. I had to change one previous test slightly *(test_one_cancellation())*, which seems in line though, with the proposed behavior. -- ___ Python tracker <https://bugs.python.org/issue40894> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
