Skip to content

Adding Websockets#54

Merged
jhjaggars merged 9 commits intoproject-receptor:masterfrom
jhjaggars:websockets
Dec 5, 2019
Merged

Adding Websockets#54
jhjaggars merged 9 commits intoproject-receptor:masterfrom
jhjaggars:websockets

Conversation

@jhjaggars
Copy link
Copy Markdown
Member

This PR introduces WebSocket support for connections.

Currently when you start a node in listening mode a WebSocket server is also started on port+1. Peers can be added by appending ws[s]:// to the front of the address.

@jhjaggars jhjaggars requested a review from matburt November 25, 2019 19:54
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
@jhjaggars jhjaggars marked this pull request as ready for review December 3, 2019 21:33
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Copy link
Copy Markdown
Member

@matburt matburt left a comment

Choose a reason for hiding this comment

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

This is really just some logging nitpicking.

Comment thread receptor/__main__.py
except Exception as e:
logger.error("An error occured while running receptor:\n%s" % (str(e),))
except Exception:
logger.exception("main: an error occured while running receptor")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we hide the reason?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The reason will be in the stack trace (which I found more helpful), but it is longer.

Comment thread receptor/messages/envelope.py Outdated
Comment thread receptor/node.py Outdated
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
@jhjaggars jhjaggars merged commit 0ae97cf into project-receptor:master Dec 5, 2019
Comment thread receptor/ws.py

class WSClient(WSBase):
async def connect(self, uri):
async with aiohttp.ClientSession().ws_connect(uri) as ws:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tower just started using aiohttp. You probably want to get a handle on that session and call await session.close() or use a context manager to handle the closing or prepared to be peppered with logger messages Unclosed client session

Comment thread receptor/ws.py
self.unregister(ws)
await asyncio.sleep(5)
logger.debug("connect: reconnecting")
self.loop.create_task(self.connect(uri))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I totally stole this recursive reconnect hotness.

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.

3 participants