Skip to content

bpo-34622: Extract asyncio exceptions into a separate module#9141

Merged
asvetlov merged 5 commits into
python:masterfrom
asvetlov:async-exceptions
Sep 11, 2018
Merged

bpo-34622: Extract asyncio exceptions into a separate module#9141
asvetlov merged 5 commits into
python:masterfrom
asvetlov:async-exceptions

Conversation

@asvetlov

@asvetlov asvetlov commented Sep 10, 2018

Copy link
Copy Markdown
Contributor

@asvetlov asvetlov requested a review from 1st1 September 10, 2018 19:41
@asvetlov asvetlov changed the title Extract asyncio exceptions into a separate module bpo-34622: Extract asyncio exceptions into a separate module Sep 10, 2018

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to set __module__ = 'asyncio' in all classes for pickle compatibility. This is needed if you want to unpickle pickles created in 3.8 in older Python versions.

Or set fake __name__ = 'asyncio' in the asyncio.exceptions module.

I don't know whether it is a good idea to move all exceptions.

Comment thread Lib/asyncio/exceptions.py
from . import base_futures


class CancelledError(concurrent.futures.CancelledError):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why define subclasses instead of just making aliases?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main motivation is to improve usability. There's no fundamental reason why these exceptions are derived from concurrent.future, it happened pretty much by accident. But the problem is that asyncio users see concurrent.future.TimeoutError exceptions occurring in their programs and have no idea why it's not asyncio.TimeoutError.

Moreover, we have a plan to make asyncio.CancelledError a BaseException in a follow up PR.

Comment thread Lib/asyncio/futures.py Outdated


def _convert_future_exc(exc):
if type(exc) is concurrent.futures.CancelledError:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you cache the type(exc) call?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@asvetlov

Copy link
Copy Markdown
Contributor Author

Exception classes are not stored persistently.
Pickling an exception, e.g. for passing into process worker by multiprocesses call should work pretty fine.

We are creating new exception classes anyway, no need to use tricks for very rare and weird backward compatibility use cases.

A code from python < 3.8 will be unpickled into concurrent.futures exceptions. Not ideal but still not the world breakage.

@1st1

1st1 commented Sep 11, 2018

Copy link
Copy Markdown
Member

@asvetlov I agree, feel free to merge as soon as CI is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants