search

sanic-org / sanic

Accelerate your web app development | Build fast. Run fast.
https://sanic.dev
MIT License
18.01k stars 1.54k forks source link

ImportError: cannot import name 'CLOSED' from 'websockets.connection' #2733

Open vcidst opened 1 year ago

vcidst commented 1 year ago

Is there an existing issue for this?

Describe the bug

We're using Sanic 21.12.2 at rasa and notice this bug whenever rasa tries to spin a sanic server,

File "/Users/zi/Work/Rasa/venv-rasa-oss-3-10-2/lib/python3.10/site-packages/sanic/mixins/startup.py", line 57, in <module>
    from sanic.server.protocols.websocket_protocol import WebSocketProtocol
  File "/Users/zi/Work/Rasa/venv-rasa-oss-3-10-2/lib/python3.10/site-packages/sanic/server/protocols/websocket_protocol.py", line 3, in <module>
    from websockets.connection import CLOSED, CLOSING, OPEN
ImportError: cannot import name 'CLOSED' from 'websockets.connection' (/Users/zi/Work/Rasa/venv-rasa-oss-3-10-2/lib/python3.10/site-packages/websockets/connection.py)

It seems like https://github.com/sanic-org/sanic/pull/2609 addresses it but these changes are not available to the branch for version 21 21.12LTS.

Code snippet

No response

Expected Behavior

I would have expected to not see any error when starting the sanic server. I get expected behaviour when using sanic version 22.12.0 and 23.3.0

How do you run Sanic?

Sanic CLI

Operating System

MacOS

Sanic Version

Sanic 21.12.2; Routing 0.7.2

Additional context

No response

aaugustin commented 1 year ago

I submitted changes to Sanic in November to prevent this issue: #2609.

If you're sticking to an old version of Sanic, you have to stick to an old version of websockets too, specifically <11.0.

Indeed, Sanic used to rely on implementation details of websockets, before I submitted that MR to use the public API.

aaugustin commented 1 year ago

If Sanic's stability policies require fixing it for 21.12 LTS, I would recommend one of:

Alternatively, it would be technically possible to address this in websockets by restoring aliases for the objects that Sanic imports. However, this sets a bad precedent for the distinction between the public API and the rest of the implementation. I'd rather avoid it.

ahopkins commented 1 year ago

No, we should have the constraint. That is absolutely what the intent was, and if it's not there, then I will add it. I'm away from my computer for a few days, so you will manually need to constrain your env.