Creating Websocket Handler #24
Conversation
…re out what's wrong with integration tests
jhileman07
force-pushed
the
jhileman/7-python-util-websocket-handler
branch
from
c2b6e75
to
9d9d908
Compare
| websocket_client.connect() | ||
|
|
||
| # Wait for the connection to be established | ||
| asyncio.get_event_loop().run_until_complete(asyncio.sleep(1)) |
There was a problem hiding this comment.
I feel like the client should be dealing with this internally. Maybe some sort of flag for when connect is called, and then dependent functions will spin if the flag is set but the connection hasn't been established
There was a problem hiding this comment.
added a self._connected boolean flag to handle internally
There was a problem hiding this comment.
You should expose connected via a property so you can spin like so
while not ws_client.connected:
pass
echavemann
force-pushed
the
jhileman/7-python-util-websocket-handler
branch
from
030a081
to
db3ca15
Compare
| from typing import Optional | ||
| import threading | ||
|
|
||
| logging.basicConfig(level=logging.INFO) |
There was a problem hiding this comment.
Logging should be configured in an application entrypoint, not in library code --- this would break previous configurations
| if isinstance(message, bytes): | ||
| message = message.decode("utf-8") | ||
| logger.info(f"Received message: {message}") | ||
| self.received_messages.append(message) |
There was a problem hiding this comment.
This should just be empty... exposing received_messages like this will lead to concurrency errors
Let clients impl their own message handling
| logger.error(f"WebSocket connection error: {error}") | ||
| # reconnect logic | ||
| if self._loop: | ||
| self._loop.call_later(self.reconnect_interval, self.connect) |
There was a problem hiding this comment.
This will spawn a new thread and loop without cleaning up the existing ones
| self.url: str = url | ||
| self.websocket: Optional[websockets.WebSocketClientProtocol] = None | ||
| self.reconnect_interval: int = reconnect_interval |
There was a problem hiding this comment.
these should all be private
if you want to expose url or reconnect interval, use a property
Description
We need a handler to run a websocket to be able to receive messages.
Testing
Attempted to create unit tests for certain functions and an integration test.
Impact
A websocket is necessary to listen to messages.
Other
Must ensure that the way async was handled won't affect the codebase.