python-trio / trio-websocket

WebSocket client and server implementation for Python Trio
MIT License
70 stars 26 forks source link

Remove python 3.5 support. Fixes #112 #138

Closed AutomatedTester closed 2 years ago

AutomatedTester commented 4 years ago

Failures due to #137

belm0 commented 4 years ago

this would also need:

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 188


Totals Coverage Status
Change from base Build 187: -0.008%
Covered Lines: 496
Relevant Lines: 518

💛 - Coveralls
AutomatedTester commented 4 years ago

Done

AutomatedTester commented 4 years ago

Hey @belm0 is there anything left to do to get this merged?

belm0 commented 4 years ago

remove async_generator dependency as mentioned in #112

It means removing all references to this 3rd party module:

https://github.com/HyperionGray/trio-websocket/blob/d878659a753bb4ecc97f5b39485b9af77fdf639a/trio_websocket/_impl.py#L9

https://github.com/HyperionGray/trio-websocket/blob/be300518ff33b18d8c2d7a8b85ad3bd15023457a/tests/test_connection.py#L40

In Python 3.6, the yield keyword can be used normally within an async generator, and so async_generator and yield_ are not needed. And use asynccontextmanager from the stdlib.

ehaas commented 4 years ago

In Python 3.6, the yield keyword can be used normally within an async generator, and so async_generator and yield_ are not needed. And use asynccontextmanager from the stdlib.

FYI asynccontextmanager requires 3.7 -- see https://docs.python.org/3/library/contextlib.html#contextlib.asynccontextmanager "New in version 3.7."

belm0 commented 4 years ago

FYI asynccontextmanager requires 3.7 -- see https://docs.python.org/3/library/contextlib.html#contextlib.asynccontextmanager "New in version 3.7."

Sorry, my oversight!

For this PR, removing use of the @async_generator decorator and replacing yield_ with yield look straightforward. I'd like to do that, and restore the dependency on async_generator which is needed for the asynccontextmanager shim.

(Later we can consider implementing the async context manager manually to remove the dependency on this lib.)

ehaas commented 4 years ago

In case it's helpful - I extracted asynccontextmanager from latest cpython and removed some unnecessary abstraction. Tested in Python 3.6.11 and it seems to work. Only issue is that by default my linter complained about Async context manager 'async_generator' doesn't implement __aenter__ and __aexit__ (not-async-context-manager) when using it to wrap a function. The code itself works though.

from functools import wraps

class _AsyncGeneratorContextManager:
    """Helper for @asynccontextmanager."""

    def __init__(self, func, args, kwds):
        self.gen = func(*args, **kwds)
        self.func, self.args, self.kwds = func, args, kwds
        # Issue 19330: ensure context manager instances have good docstrings
        doc = getattr(func, "__doc__", None)
        if doc is None:
            doc = type(self).__doc__
        self.__doc__ = doc
        # Unfortunately, this still doesn't provide good help output when
        # inspecting the created context manager instances, since pydoc
        # currently bypasses the instance docstring and shows the docstring
        # for the class instead.
        # See http://bugs.python.org/issue19404 for more details.

    async def __aenter__(self):
        try:
            return await self.gen.__anext__()
        except StopAsyncIteration:
            raise RuntimeError("generator didn't yield") from None

    async def __aexit__(self, typ, value, traceback):
        if typ is None:
            try:
                await self.gen.__anext__()
            except StopAsyncIteration:
                return
            else:
                raise RuntimeError("generator didn't stop")
        else:
            if value is None:
                value = typ()
            # See _GeneratorContextManager.__exit__ for comments on subtleties
            # in this implementation
            try:
                await self.gen.athrow(typ, value, traceback)
                raise RuntimeError("generator didn't stop after athrow()")
            except StopAsyncIteration as exc:
                return exc is not value
            except RuntimeError as exc:
                if exc is value:
                    return False
                # Avoid suppressing if a StopIteration exception
                # was passed to throw() and later wrapped into a RuntimeError
                # (see PEP 479 for sync generators; async generators also
                # have this behavior). But do this only if the exception wrapped
                # by the RuntimeError is actully Stop(Async)Iteration (see
                # issue29692).
                if isinstance(value, (StopIteration, StopAsyncIteration)):
                    if exc.__cause__ is value:
                        return False
                raise
            except BaseException as exc:
                if exc is not value:
                    raise

def asynccontextmanager(func):
    @wraps(func)
    def helper(*args, **kwds):
        return _AsyncGeneratorContextManager(func, args, kwds)
    return helper
belm0 commented 3 years ago

@AutomatedTester would you open this PR for maintainer edits?

I have some follow-up commits pending for changes in master and using native async generators.

Screen Shot 2020-11-21 at 3 49 07 PM

AutomatedTester commented 3 years ago

@AutomatedTester would you open this PR for maintainer edits?

I have some follow-up commits pending for changes in master and using native async generators.

Screen Shot 2020-11-21 at 3 49 07 PM

It showing as already done