Skip to content
This repository has been archived by the owner on Apr 22, 2024. It is now read-only.

Improve websocket connection #432

Closed
wants to merge 1 commit into from
Closed

Conversation

macartur
Copy link
Contributor

This commit will improve the websocket to use gevent + gevent-websocket to avoid
the error KeyError: 'Session is disconnected'
Fix #416

This commit will improve the websocket to use gevent + gevent-websocket to avoid
the error KeyError: 'Session is disconnected'
Fix kytos#416
@cemsbr
Copy link
Contributor

cemsbr commented May 22, 2017

I tried to use eventlet, but it was not logging messages sent outside the eventlet. Did you check whether all messages are being sent?

Besides, I'm working on a huge logging refactoring removing the websocket buffer and I think we should wait my PR to discuss a possible merge (in the next day or two).

Copy link
Contributor

@cemsbr cemsbr left a comment

Choose a reason for hiding this comment

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

My comments are in the conversation.

@beraldoleal
Copy link
Member

Since that @cemsbr is working on this I will wait until his PR. Also I noticed two more dependencies. We have to try to avoid adding more dependencies.

@cemsbr
Copy link
Contributor

cemsbr commented May 30, 2017

To have WebSockets instead of long polling, we must use gevent (and gevent-websocket) or eventlet in flask-socketio. Both lead to strange behavior when logging from outside flask. This is because they make flask run asynchronously, and our code is not async. I haven't found a simple solution for that.

I suggest keeping the long-polling approach until we change our architecture. Performance is not an issue right now.

@beraldoleal
Copy link
Member

@macartur ?

@diraol
Copy link
Contributor

diraol commented Jun 13, 2017

@cemsbr this PR should be considered even after your PR about this?

@cemsbr
Copy link
Contributor

cemsbr commented Jun 13, 2017

I'm closing this in favor of #451, as discussed with @macartur.

@cemsbr cemsbr closed this Jun 13, 2017
@macartur macartur deleted the issue-#416 branch July 11, 2017 13:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants