[Pyamqp] Revert back to websocket-client #26787
Conversation
kashifkhan
requested review from
annatisch,
swathipil and
l0lawrence
as code owners
ghost
added
the
|
/azp run python - eventhub - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -149,6 +149,36 @@ async def send_frame(self, channel, frame, **kwargs): | |||
| await self.write(data) | |||
| # _LOGGER.info("OCH%d -> %r", channel, frame) | |||
|
|
|||
| class AsyncTransport( | |||
There was a problem hiding this comment.
can we copy changes to SB pyamqp as well?
There was a problem hiding this comment.
yes these changes will have to go back there too
There was a problem hiding this comment.
sorry, I meant can you copy them over in this PR? 🙂
or do we want to do that later?
There was a problem hiding this comment.
lol my reply wasnt clear, Ill do it in this PR itself :)
|
|
||
| async def _read(self, n, buffer=None, **kwargs): # pylint: disable=unused-argument | ||
| async def _read(self, n, buffer=None, **kwargs): # pylint: disable=unused-arguments |
There was a problem hiding this comment.
might be a typo and need to be
| async def _read(self, n, buffer=None, **kwargs): # pylint: disable=unused-arguments | |
| async def _read(self, n, buffer=None, **kwargs): # pylint: disable=unused-argument |
There was a problem hiding this comment.
not sure if this will be a lot of work, but would you be able to compare with the sync transport/websocket to see if there were changes that need to be copied over here? I think I may have added some mypy/pylint things?
There was a problem hiding this comment.
not a lot of work, I just brought in the old WebSocketTransportAsync and the current portions untouched. Any re-shuffling will keep all the pylint/mypy work. Will take a look at the WebSocketTransportAsync though
|
API change check APIView has identified API level changes in this PR and created following API reviews. |
|
/azp run python - eventhub - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - eventhub - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - eventhub - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - eventhub - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run python - eventhub - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Due to issues with running websocket-client in async mode, we decided to stay with aiohttp for async. We were able to use heartbeat from aiohttp and timeouts to mitigate the bugs. |
|
@kashifkhan im having the same issue, can you elaborate on what you did with aiohttp, which timeouts did you use? |
@gil-cohen I passed in heartbeat parameter in ws_connect with a value of 10 seconds. That was enough time for the library to determine that the underlying connection has been lost. Additionally you can take a look at these 2 PRs where you can see the changes and some of the exceptions that I am now addressing #26856 |
aiohttp websocket has 2 bugs when a connection is abruptly cut:
websocket send does not throw an exception -> issue -> known since 2017
websocket receive gets stuck if network is suddenly disconnected -> PR -> unmerged since July 2021
Reverting back to websocket-client in async as that is what most are using. There is another async websocket client but it doesnt support proxies.