Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redis / Eventlet / Flask-SocketIO #618

Closed
ocervell opened this issue Jan 15, 2018 · 29 comments
Closed

Redis / Eventlet / Flask-SocketIO #618

ocervell opened this issue Jan 15, 2018 · 29 comments

Comments

@ocervell
Copy link

ocervell commented Jan 15, 2018

I am unable to make the combination (Redis / Eventlet / SocketIO) work.

I have forked your Flask-SocketIO-Chat to demonstrate this bug here (branch:eventlet-redis-bug) .

Adding message_queue='redis://' to SocketIO init, the chat is not working properly anymore (it seems messages are not flowing through the websocket). Redis is running properly on localhost:6379.

Here is the sample init code:

app/__init__.py

from flask import Flask
from flask_socketio import SocketIO
import eventlet
eventlet.monkey_patch(socket=True)

# No Redis (works)
# socketio = SocketIO(async_mode='eventlet')

# With Redis (bug)
socketio = SocketIO(message_queue='redis://', async_mode='eventlet')


def create_app(debug=False):
    """Create an application."""
    app = Flask(__name__)
    app.debug = debug
    app.config['SECRET_KEY'] = 'gjr39dkjn344_!67#'

    from .main import main as main_blueprint
    app.register_blueprint(main_blueprint)

    socketio.init_app(app)
    return app

chat.py

from app import create_app, socketio
app = create_app(debug=True)

if __name__ == '__main__':
   socketio.run(app, debug=True)

Steps to reproduce:

git clone https://github.com/ocervell/Flask-SocketIO-Chat.git
cd Flask-SocketIO-Chat
git checkout eventlet-redis-bug
pip install -r requirements.txt
FLASK_APP=chat.py flask run
@ocervell
Copy link
Author

I figured out what's causing the issue, the socketio.run(app, debug=True) in chat.py. When removing debug=True it works.

@miguelgrinberg
Copy link
Owner

There is another problem in your code. When you initialize a SocketIO instance with a message queue but without an app instance, you are creating a "passive" instance that can only emit, but is not a server. This is used for external processes that want to emit through a Flask-SocketIO server.

Since in this case we are using a delayed initialization due to the use of the app factory function, the message_queue argument should be given in the init_app() call inside the factory.

socketio = SocketIO(async_mode='eventlet')

def create_app(debug=False):
    # ...
    socketio.init_app(app, message_queue='redis://')
    return app

@ocervell
Copy link
Author

Thanks.

Will the following also work (everything in init_app) ?

socketio = SocketIO()
...
socketio.init_app(app, async_mode='eventlet', message_queue='redis://')

so that I can follow my existing implementation for registering extensions ?

@miguelgrinberg
Copy link
Owner

Yes, that is actually better when you are using the delayed initialization style.

@ocervell
Copy link
Author

ocervell commented Jan 15, 2018

Thanks, got it to work on my MacOSX with redis-server running.

On Windows (Cygwin), no luck. It works fine without the message_queue argument. Might be linked to #557.

As soon as I add message_queue, the chat stops working (messages stop flowing) and there are no errors in my logs:

  • Redis running OK (as a Windows service)
  • Server running OK
  • UI using Redis for caching functions OK

If you're interested, here is the project.
Navigate to localhost:5000/chat to reproduce.

@miguelgrinberg
Copy link
Owner

Can you debug it a bit to see if the problem is in writing to redis or reading from it?

@ocervell
Copy link
Author

ocervell commented Feb 3, 2018

I just gave up on making it work on Windows and switched to an Ubuntu box. It works fine there as well... I'm pretty busy at the moment but you can always clone the fork I linked to reproduce on Windows.

@viggyfresh
Copy link

@miguelgrinberg sorry to resuscitate something so old, but not putting the initialization arguments into the call to init_app just got me (and this issue had the fix)! would you accept a PR making this explicit in the documentation? and would you have a suggestion as to where this note would fit best? I was thinking in the Initialization section

@miguelgrinberg
Copy link
Owner

@viggyfresh the way to use init_app() is standard for all Flask extensions, not sure it is worth documenting here as well, more so considering that init_app() is almost always used along with a application factory, so a realistic example would not be just a short snippet.

@viggyfresh
Copy link

Sounds good, no problem

@nirvana-msu
Copy link

@viggyfresh the way to use init_app() is standard for all Flask extensions, not sure it is worth documenting here as well, more so considering that init_app() is almost always used along with a application factory, so a realistic example would not be just a short snippet.

I just spent hours debugging this until I stumbled upon this thread. I must say this is very confusing and needs to be documented. Application factory pattern is merely a way to plug the app into an instantiated extension at a later time. There is nothing at all that would hint a user that a certain argument can only be provided during init_app but not before. In fact that's the only Flask extension I'm aware of where I cannot provide an argument beforehand. In hindsight it probably makes sense (given that instantiating without an app is a valid use-case here - unlike with any other Flask extension), but that requires a rather deep understanding of the internals. In my view this needs to be clearly articulated in the documentation.

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Jul 1, 2020

that a certain argument can only be provided during init_app but not before

This is incorrect. The SocketIO extension works in the same way as most other extensions. You can initialize it in a single step:

    socketio = SocketIO(app)

or in two steps:

    socketio = SocketIO()

    def create_app():
        app = Flask(__name__)
        socketio.init_app(app)
        return app

Both work, so I still don't see what was the source of confusion.

The only unusual thing about the SocketIO extension is that it has this "passive" mode, where it is initialized without a Flask app. But the same rules apply. You can initialize a passive SocketIO object in one or two steps.

@nirvana-msu
Copy link

Sure, that's all clear. Confusion comes from the fact that one could think that the following is a valid approach:

socketio = SocketIO(message_queue='amqp://')

def create_app():
    app = Flask(__name__)
    socketio.init_app(app)
    return app

There's nothing in any of the official docs anywhere that suggests that any extra arguments (message_queue in this case) must be passed to init_app when using application factory pattern, rather than when instantiating the extension. But this fails in this case (with no obvious indication as to why).

Passing arguments to extensions is not too common (as they're usually configured through Flask config variables) - but when the arguments are provided directly, in every extension I've come across it doesn't matter if the arguments are provided when instantiating the extension, or later when binding to the app. That's because they just merge both sets of arguments, and then initialize the extension. SocketIO is somewhat unique in a sense that message_queue argument can only be provided during init_app and not before.

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Jul 1, 2020

There's nothing in any of the official docs anywhere that suggests that any extra arguments (message_queue in this case) must be passed to init_app when using application factory pattern, rather than when instantiating the extension.

Yes, this is because the extension works as all other extensions, you can provide args in either place. You must have had a very specific issue that you are not describing. The code example that you shared where message_queue is given in the constructor and app is given in the init_app() call seems to work fine. (I used redis instead of amqp to test this).

in every extension I've come across it doesn't matter if the arguments are provided when instantiating the extension, or later when binding to the app.

Correct. The goal is that this extension works in the same way. If you have a specific case in which it doesn't, I can fix it.

SocketIO is somewhat unique in a sense that message_queue argument can only be provided during init_app and not before.

You need to show me an actual example in which this is broken, because a simple test based on your example worked fine for me. I would not recommend people to split the args in two places though, and that is why I commented against it in this issue.

@MarianoDel
Copy link

Hi Miguel, I write in other place a question but I think is more relevent here.
I use pyaudio with callback to emit websockets, some emits are from simple app buttons and other is from pyaudio callback audio data. All works fine if I use the development aproach, flask flask-socketio pyaudio werkzeug, but I need better performance for the audio packets, so I install eventlet as you recommend on flask-socketio page.
As soon I installed enventlet the emits for buttons works ok, but emiting inside the callback of pyaudio is not.
All the emits inside pyaudio callback are not going out, even if is not audio data (I tested the other buttons emits from the insede the callback).
So my problem is that I can get emits from inside a pyaudio callback using eventlet, there is some approach that I can test to get this thing going? Regards

@miguelgrinberg
Copy link
Owner

Eventlet is unlikely to work well with pyaudio, which is a C package. Not much we can do about it, greenlets are kinda picky, they work well with Python code but not so much with native code.

@viggyfresh
Copy link

There's nothing in any of the official docs anywhere that suggests that any extra arguments (message_queue in this case) must be passed to init_app when using application factory pattern, rather than when instantiating the extension.

Yes, this is because the extension works as all other extensions, you can provide args in either place. You must have had a very specific issue that you are not describing. The code example that you shared where message_queue is given in the constructor and app is given in the init_app() call seems to work fine. (I used redis instead of amqp to test this).

in every extension I've come across it doesn't matter if the arguments are provided when instantiating the extension, or later when binding to the app.

Correct. The goal is that this extension works in the same way. If you have a specific case in which it doesn't, I can fix it.

SocketIO is somewhat unique in a sense that message_queue argument can only be provided during init_app and not before.

You need to show me an actual example in which this is broken, because a simple test based on your example worked fine for me. I would not recommend people to split the args in two places though, and that is why I commented against it in this issue.

hi @miguelgrinberg - this hit us a really long time back, so I cannot remember the exact specifics, but I believe a situation where the arguments don't work uniformly in both spots is the following:

  • You have a plain-ol-Flask API server and a separate Flask-SocketIO chatserver
  • You are adding the message_queue keyword argument in the SocketIO constructor (and not in init_app)
  • You are hitting a Flask route and you want to trigger a SocketIO emit
  • You are using eventlet

In this case, IIRC, what would happen is we'd hit the Flask route, but nothing would ever get sent through the message queue (and thus no emit). Simply moving the message_queue keyword argument to the init_app call fixed the issue - and that's when I left the comment higher up in the thread 😄

Not sure if this is helpful!

@MarianoDel
Copy link

MarianoDel commented Jul 12, 2020 via email

@miguelgrinberg
Copy link
Owner

Communication via socket should work fine, and would allow you to use websocket on the server, so yes, that is a good option.

@mgd722
Copy link

mgd722 commented Nov 5, 2020

The code example that you shared where message_queue is given in the constructor and app is given in the init_app() call seems to work fine.

@miguelgrinberg I can confirm that this does not work; you must put message_queue='redis://' in the socketio.init_app call to get it to run. If message_queue is given in the constructor, it will fail silently. Here is a cloneable example using your websockets chat app. Other than adding a redis dependency and eventlet monkey patching, this is the only thing I changed. Running Python 3.6.6. I tried this configuration with both eventlet and gevent and neither worked.

@miguelgrinberg
Copy link
Owner

@mgd722 That's okay, it was not a usage that I intended to support anyway. What I'm going to do when I get a moment is make sure an error is displayed if the extension is used in that way.

@mgd722
Copy link

mgd722 commented Nov 5, 2020

That makes sense. Most of my confusion came from your previous comment "...the extension works as all other extensions, you can provide args in either place." An error to tell users when it's configured incorrectly sounds like a perfectly appropriate solution. For simplicity for myself and other future Googlers, this is the working configuration:

from flask import Flask
from flask_socketio import SocketIO

# does not work
# socketio = SocketIO(message_queue='redis://')

# does work
socketio = SocketIO()


def create_app(debug=False):
    """Create an application."""
    # ...your create app stuff goes here

    # does not work
    # socketio.init_app(app)

    # does work
    socketio.init_app(app, message_queue='redis://')

    return app

@skamensky
Copy link

This hit me as well. I was debugging this for a few days and it took me a while to nail down that it was related to the fact that I was using pubsub (in all of my efforts to reproduce the issue I naively left out the message_queue argument ).

The reason I put the message_queue string in the constructor of the SocketIO class to begin with was because I wanted a "passive" server of the type that @miguelgrinberg indicates in this quote

When you initialize a SocketIO instance with a message queue but without an app instance, you are creating a "passive" instance that can only emit, but is not a server. This is used for external processes that want to emit through a Flask-SocketIO server.

And I figured the same SocketIO instance could be used for both the "emitting only" SocketIO instance and the actual SocketIO server in difference contexts.

The solution is to create to separate instances, one for "emitting only" (which I just create adhoc and let the garbage collector remove later) and one for the actual application server (which lives in memory for as long the application is alive).

I understand that maybe the letter of the law may say this does not need to be documented, but I think many users of this library would benefit from a brief mention of this "two instance" strategy and in actuality it may end up saving many hours of debugging.

@miguelgrinberg
Copy link
Owner

@skamensky What you are doing is not a good idea. The "active" Socket.IO instance is a superset of the "passive" one, so there is no need to create a second instance if you want to emit. Just use the first one.

All you need to do is to put the arguments to your active Socket.IO instance in one place. Either in the SocketIO() constructor, or in the init_app() method. Splitting the arguments across these two is what sometimes can cause issues.

@skamensky
Copy link

skamensky commented Nov 13, 2021

@miguelgrinberg , the issue is that the SocketIO instance that is connected to flask is only instantiated (i.e. the factory function is called, all of the flask application settings are configured, and the socketio.run method is called) within the web application's context.

Other non-web processes and threads are also able to communicate to the clients and create a new ServerIO instance which has nothing to do with the flask application. So the "superset" is not always available when the "passive" version of the SocketIO instance is required. Does that make sense, or am I missing something?

Thank you very much for all of your help!

@miguelgrinberg
Copy link
Owner

miguelgrinberg commented Nov 13, 2021

@skamensky sorry, but no, this does not make sense to me.

To be able to emit, you need the "active" Socket.IO instance created and running. If this instance isn't running, then there is no way for clients to connect, or for any threads to emit.

Other non-web processes and threads

For non-web processes I agree 100%. This is the intended use of the "passive" Socket.IO instance.

Non-web threads living in the same process as the active instance do not need a passive instance. Just import the active instance and use it to emit to clients directly.

@skamensky
Copy link

@miguelgrinberg ,

I think we're on the same page and there's some sort of semantic misunderstanding.
I am using this library to communicate with clients in both and web-processes and non-web processes (i.e. functions that are not guaranteed to have a flask global application context defined).

I will therefore take this quote as a blessing 😊

For non-web processes I agree 100%. This is the intended use of the "passive" Socket.IO instance.

And revisit this one as penance for my sin of disobedience if I ever happen in to unexplained silent failures 🙃.

Splitting the arguments across these two is what sometimes can cause issues.

Again, cheers for all the help! You are a true champ.

@Naught0
Copy link

Naught0 commented Dec 15, 2021

# does not work
# socketio = SocketIO(message_queue='redis://')

# does work
socketio = SocketIO()

#618 (comment)

I wish I had come across this hours ago! A callout in the docs about this would be immensely helpful for relative newbies like myself attempting to implement Flask-SocketIO using the app factory pattern.

Glad this solved it though 🍻

@miguelgrinberg
Copy link
Owner

This is a duplicate of #1130 which is now fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants