Fix from websockets.(asyncio|sync).router import * without werkzeug

[hroncok]

This is a followup for 3128f56

Also, skip test_router_supports_unix_sockets without werkzeug.

[aaugustin]

Thank you. At first sight this looks correct. I'll take a closer look and merge ASAP.

[hroncok]

Amended to also skip the test for the sync variant.

[aaugustin]

The failed test run above is unrelated to your changes; it's backwards-incompatible changes in mypy :-(

[aaugustin]

mypy doesn't like the override :-(

I see at least two options:

1. Move the Router class outside of the try / except ImportError block. Indeed, defining the class doesn't depend on werkzeug, only running it.
2. # type: ignore.

[hroncok]

Which one do you prefer?

[aaugustin]

Skipping tests that depend on werkzeug when werkzeug isn't installed is clearly correct.

I'm less clear about what needs to be done for the Router class; after further investigation, I believe the problem I must solve is this one:

>>> from websockets.asyncio.router import *
Traceback (most recent call last):
  File "<python-input-0>", line 1, in <module>
    from websockets.asyncio.router import *
AttributeError: module 'websockets.asyncio.router' has no attribute 'Router'. Did you mean: 'route'?

Note that it would not solve the problem described in fedora-eln/eln#276 because the dependency on werkzeug was explicitly declared and it will still be there regardless of the changes we're discussing here.

---

While we're discussing packaging, if Fedora wants to package websockets officially, it would be good to follow these guidelines e.g. to make sure that the code you're packaging is actually the code that I'm releasing: https://websockets.readthedocs.io/en/stable/project/contributing.html#packaging

I am also puzzled by the decision to run a random subset of tests — why run the asyncio tests but not the sync tests? — and to skip failing tests — this defeats the point of running tests in the first place, doesn't it? It's burning CPU and creating manual work to add arbitrary tests to the list of failing tests when a build server gets a load spike while running the tests and a test fails randomly; I don't see how that creates value.

[aaugustin]

Router is in __all__ and is documented because it's the default value of create_router. However, it's really at the frontier between public and private API. Its methods aren't documented; if a user wants to override it, they have to read the source and deal with the consequences. That's why I don't think the nice error message that we have for route and unix_route is really needed here. I'll just move Router at the toplevel.

[hroncok]

I'm less clear about what needs to be done for the Router class; after further investigation, I believe the problem I must solve is this one:

>>> from websockets.asyncio.router import *
Traceback (most recent call last):
  File "<python-input-0>", line 1, in <module>
    from websockets.asyncio.router import *
AttributeError: module 'websockets.asyncio.router' has no attribute 'Router'. Did you mean: 'route'?

My change fixes the issue but breaks the typing check. Always defining the class but raising from __init__ when workzeug is not importable is also a solution.

---

Note that it would not solve the problem described in fedora-eln/eln#276 because the dependency on werkzeug was explicitly declared and it will still be there regardless of the changes we're discussing here.

That's why I proposed to remove that declaration together with this change, see https://src.fedoraproject.org/rpms/python-websockets/pull-request/12 (churchyard is my username in Fedora).

While we're discussing packaging, if Fedora wants to package websockets officially, it would be good to follow these guidelines e.g. to make sure that the code you're packaging is actually the code that I'm releasing: https://websockets.readthedocs.io/en/stable/project/contributing.html#packaging

I am here as an innocent drive-by contributor; the Fedora websockets package is not packaged by me. I merely wanted to drop the dependency declared by the Fedora maintainer.

I am also puzzled by the decision to run a random subset of tests — why run the asyncio tests but not the sync tests? — and to skip failing tests — this defeats the point of running tests in the first place, doesn't it?

I agree, and I also proposed to run them all: https://src.fedoraproject.org/rpms/python-websockets/pull-request/14

Also, run them on all architectures: https://src.fedoraproject.org/rpms/python-websockets/pull-request/13

[aaugustin]

I don't think it makes sense for Fedora to run the tests but I've gotten used to disagreeing with distro maintainers on this :-)

[hroncok]

What sort of verification would we run instead? E.g. when we update to a new Python version before you add it to your CI, how do we verify websockets isn't entirely broken?

[aaugustin]

Fair point. I don't have a better answer to this than "run the tests". This allows you to validate automatically.

[aaugustin]

In the case of Python 3.14, interestingly, the code is fine but the tests need a fix to pass without warnings :-)

[hroncok]

Indeed, there is:

tests/legacy/utils.py:28: 526 warnings
  /builddir/build/BUILD/python-websockets-15.0.1-build/websockets-15.0.1/tests/legacy/utils.py:28: DeprecationWarning: 'asyncio.iscoroutinefunction' is deprecated and slated for removal in Python 3.16; use inspect.iscoroutinefunction() instead
    if asyncio.iscoroutinefunction(test):

[aaugustin]

Fixed in main now :-)

[hroncok]

Thanks.