Skip to content

asyncio.transports.BaseTransport: Add payload to close #103338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
miili opened this issue Apr 7, 2023 · 7 comments
Closed

asyncio.transports.BaseTransport: Add payload to close #103338

miili opened this issue Apr 7, 2023 · 7 comments
Labels
stdlib Python modules in the Lib dir topic-asyncio type-feature A feature request or enhancement

Comments

@miili
Copy link

miili commented Apr 7, 2023

Feature or enhancement

Calling Transport.close maybe calls connection_lost when closed.

class Protocol(asyncio.Protocol):
    def connection_lost(self, exc: Exception | None = None) -> None:
        ...

To e.g. gracefully close connections it would be helpful to transport an exception payload to connection_lost. Which if exc is present, will always call Protocol.connection_lost.

Pitch

Implement Protocol.close to take an exception argument:

class BaseTransport:
    def close(self, exc: Exception | None = None) -> None:
        ...
        if exc:
            protocol.connection_lost(exc)

This will yield more control and guards to closing the connection.

@miili miili added the type-feature A feature request or enhancement label Apr 7, 2023
@arhadthedev arhadthedev added the stdlib Python modules in the Lib dir label Apr 7, 2023
@github-project-automation github-project-automation bot moved this to Todo in asyncio Apr 7, 2023
@kumaraditya303
Copy link
Contributor

What's the use case for it?

@miili
Copy link
Author

miili commented Apr 9, 2023

Explicit control over why a Transport.close was called and react distinctively.

@gvanrossum
Copy link
Member

Explicit control over why a Transport.close was called and react distinctively.

I'm sorry, but you have to give more detail about the use case you have in mind.

@miili
Copy link
Author

miili commented Apr 14, 2023

Sure thing. Following the example from https://docs.python.org/3/library/asyncio-protocol.html#connecting-existing-sockets

class IntentionalClose(Exception):
    ...


class ClientProtocol(Protocol):
    def __init__(self, connection_lost_cb: Callable) -> None:
        self.connection_lost_callback = connection_lost_cb
        super().__init__()

    def connection_lost(self, exc: Exception | None) -> None:
        # Here exc is None when the *remote* closes gracefully with EOF
        if exc is IntentionalClose:
            return
        self.connection_lost_callback()


async def main() -> None:
    def reconnect() -> asyncio.Task:
        return asyncio.create_task(connect)

    async def connect() -> None:
        loop = asyncio.get_running_loop()
        transport, protocol = await loop.create_connection(
            lambda: ClientProtocol(reconnect), "127.0.0.1", 8888
        )
        await asyncio.sleep(20.0)
        transport.close(IntentionalClose)

This interaction is explicit and avoids the connection_lost is called with None. Which can also be the case for an EOF close from the host site.
@gvanrossum Let me know if this makes the use case more clear.

@gvanrossum
Copy link
Member

gvanrossum commented Apr 15, 2023

Wait, is this connected to gh-103436? [Edit: No]

@gvanrossum
Copy link
Member

That example is incomplete, but more seriously, I don't think that it counts as a "use case". A use case is typically a story (not code!) about a real-world problem that you experienced that leads you to propose the new feature, because the problem cannot be solved without the new feature (or the solution is complicated, or particularly error-prone).

It looks to me like you want to distinguish in your protocol between a clean disconnect by the other end of the socket, and a transport.close() call. But why? What does your application do that requires the protocol to make this distinction?

@kumaraditya303
Copy link
Contributor

I don't want to further extend the transport implementation without a strong use-case hence closing.

@kumaraditya303 kumaraditya303 closed this as not planned Won't fix, can't repro, duplicate, stale Jul 16, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in asyncio Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir topic-asyncio type-feature A feature request or enhancement
Projects
Status: Done
Development

No branches or pull requests

4 participants