search

fanout / django-eventstream

Server-Sent Events for Django
MIT License
638 stars 84 forks source link

Add the GripMiddleware? #95

Closed nerdoc closed 1 year ago

nerdoc commented 2 years ago

Just asking - maybe I don't understand, or you have a bug in the main README:

you write in the Setup with Channels section:

Add the GripMiddleware:
...

But there is no grip whatsoever installed (in the steps above).

Is this necessary?

nerdoc commented 2 years ago

Maybe you could add a Django compatibility hint. even the demo asgi.py in the readme is not usable any more since Django 4.0. django.conf.urls.url was deprecated since Django 3.0, and is removed in 4.0.

jkarneges commented 1 year ago

Technically it's not necessary to update MIDDLEWARE when using Channels, because Channels ignores MIDDLEWARE and django-eventstream explicitly applies GripMiddleware internally. However, I am hoping that someday Channels (or async Django) will be compatible with regular middleware, and when/if that happens we can change django-eventstream to not explicitly apply the middleware and instead rely on the user having properly set up their MIDDLEWARE.

Thanks for the tip about Django 4 compatibility.

jkarneges commented 1 year ago

Tweaked the readme/examples.

nerdoc commented 1 year ago

Maybe another hint here would be cool. As newcomer I just think that django_grip is another package I have to install, and the README just forgot to tell me about that. Maybe add a sentence like "This middleware is included in django-eventstream". Or am I wrong?

jkarneges commented 1 year ago

Not a bad idea. I've updated the readme.