Adding the event source method to NinjaAPI and Router (SSE)

[rroblf01]

With this PR I propose to make it easier to use SSE in Django Ninja.

[vitalik]

Could you also add tests for async views ?

the sync mode does not make much sense actually as it can easily occupy all wsgi threads/processes

[rroblf01]

@vitalik I need to investigate, when I test with html views it works, but when I run the tests, it fails saying

"TypeError: object StreamingHttpResponse can't be used in 'await' expression"

I will continue investigating the reason.

[rroblf01]

Hello, @vitalik I have added a test to check the asynchronous endpoint.

[baseplate-admin]

One small nitpick

[baseplate-admin]

Could this be made async?

[rroblf01]

Yes, it works with async

[baseplate-admin]

My apologies, I meant can we refractor this to make it like, ( because we shouldn't allow users to lock up wsgi workers )

@api.event_source("/path")
async def event_source_operation(request):
    ...

[rroblf01]

It's possible, but Django's implementation already prevents that blocking, right?

[rroblf01]

Although a StreamingHttpResponse with an async generator will only work properly if they are using asgi instead of wsgi.
Should we still make async the default in the documentation?

[baseplate-admin]

Overall looks good, just one small change request

[baseplate-admin]

My apologies, I meant can we refractor this to make it like, ( because we shouldn't allow users to lock up wsgi workers )

@api.event_source("/path")
async def event_source_operation(request):
    ...

[baseplate-admin]

Also, i have a suggestion, should we allow only async server events? Otherwise we might get complaints in the future about this sync/async approach.

[rroblf01]

Also, i have a suggestion, should we allow only async server events? Otherwise we might get complaints in the future about this sync/async approach.

I understand that both should be allowed. Even the StreamingHttpResponse works better when it is synchronous (I imagine it is due to the implementation underneath)

[rroblf01]

Good morning @vitalik @baseplate-admin , What changes would be good to merge this PR? Do I put in the documentation that the method is asynchronous but with a warning that it should be used with asgi when it is asynchronous?

[rroblf01]

hello @vitalik @baseplate-admin How could we continue with this PR?

[baseplate-admin]

Apologies, i think @vitalik needs to give his final opinion on this PR.

Do I put in the documentation that the method is asynchronous but with a warning that it should be used with asgi when it is asynchronous?

Sounds good to me, add the comment made by vitalik, something like

Note that using synchronous SSE might block the WSGI worker, please use asynchronous SSE with ASGI worker for best performance.

[rroblf01]

I just added the notice

[baseplate-admin]

Okay, thank you.

@vitalik will make the final call on this