Skip to content
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

Make Listener covariant in the stream type variable #464

Merged
merged 2 commits into from
Oct 2, 2022
Merged

Make Listener covariant in the stream type variable #464

merged 2 commits into from
Oct 2, 2022

Conversation

mila
Copy link
Contributor

@mila mila commented Sep 19, 2022

This changes allows to properly annotate return type in a function like:

import ssl
from typing import Optional

import anyio
from anyio.abc import ByteStream, Listener
from anyio.streams.tls import TLSListener


async def create_listener(
    local_host: str,
    local_port: int,
    ssl_context: Optional[ssl.SSLContext],
) -> Listener[ByteStream]:
    listener = await anyio.create_tcp_listener(
        local_host=local_host, local_port=local_port
    )
    if ssl_context is not None:
        return TLSListener(listener, ssl_context=ssl_context)
    else:
        return listener

Without this change, the function has to be annotated as returning Union[Listener[SocketStream], TLSListener], which is long and it depends on specific implementation.

The stream type of the listeners acts covariantly. When I have a handler that expects ByteStream, I need Listener[ByteStream]. But the handler won't mind if it gets TLSStream, so I should not mind if I have Listener[TLSStream].

For comparison, callables are covariant in the return type. For the listeners, the socket acts similar as the return value of the callable - it is an output, just passed to the handler callback instead of returned directly.

Note that I did not propagate the change to anyio.streams.stapled.MultiListener. The reason is similar why mutable collections are not covariant. The following function should not accept MultiListener[TLSStream]:

def replace_listeners(multi_listener: MultiListener[ByteStream]):
    multi_listener.listeners = [anyio.create_tcp_listener(...)]

I also did a small change to anyio.abc.SocketListener. Its use of T_Stream seems unnecessary, so I got rid of it instead of replacing it by T_Stream_co.

@mila
Copy link
Contributor Author

mila commented Sep 19, 2022

The fail of pre-commit.ci is unrelated, so I addressed it in #465.

@agronholm
Copy link
Owner

Would you mind fixing the conflicts? I recently merged the 4.0-dev branch to master in preparation for a major release. I would like this change to be a part of that release.

SocketListener class speficies the type.

# Conflicts:
#	src/anyio/abc/_sockets.py
docs/versionhistory.rst Outdated Show resolved Hide resolved
For example, when a function expectes Listener[ByteStream], it will accept Listener[SocketStream] or Listener[TLSStream] too.
Copy link
Owner

@agronholm agronholm left a comment

Choose a reason for hiding this comment

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

Checked with mypy, LGTM.

@agronholm agronholm merged commit 70188d3 into agronholm:master Oct 2, 2022
@agronholm
Copy link
Owner

Thanks!

@mila
Copy link
Contributor Author

mila commented Oct 2, 2022

Thank you for merging.

BTW I am looking forward to the 4.0 release. I looked at the changelog today and found that at least three of the changes will help me somehow.

@mila mila deleted the listeners-covariant branch October 2, 2022 21:41
agronholm added a commit that referenced this pull request May 8, 2023
For example, when a function expectes Listener[ByteStream], it will accept Listener[SocketStream] or Listener[TLSStream] too.

(cherry picked from commit 70188d3)
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.

None yet

2 participants