django-eventstream
django-eventstream copied to clipboard
V6 proposition
/!\ DRAFT let me know what you think about
Hi jk, here is another pullrequest form me. In this PR I have :
-
Reorganise the code (because of the second point) put
views.py
and oldviewset.py
that nowapiview.py
in theviews
folder from which we can now import everything about views and apiview(more latter on the change in theviewset.py
which is nowapiview.py
). also move the js file becuase my chnage need a mjor vrsion bump so why not reorganise the code. -
Transform the viewset to APIView I transformed the viewset to an APIView because I believe it is clearer and easier to understand. The Django ViewSet was not the right choice for this use case, so I changed it to an APIView. As a result, it is no longer possible to register the viewset because, as DRF mentions, routers are meant for viewsets and not for APIViews.
With this change, I also add support for head and option request. To make the code more readable. Also i decide to delete the old methode that use the url path to retrive the channel id. If people would like to use the old methode, they can extend our class and do it by themselves.
You can also configure the docstring and the name of the class when using
configure_events_api_view
. This is useful when you want to use the view in the browsable API and you want to have a better description and name of the view. -
Better renderers config With the old redere config thta go through the REST_FRAMEWORK, other APIViews and ViewSets of apps was able to use
api_sse
andtext/event-stream
as a renderer. This is not the desired behavior. So by defaut the renderer for our application are define in the code of theAPIView
and if needed can be overriden in the settings with theEVENTSTREAM_RENDERER
variable. -
Better import for restframework part I added in utils a method
raise_not_found_module_error
which replace all the rest_framework part of the module when the module is not found. This is a better way to handle the error than the previous one. -
Including tests I modify how test are wokring by using only
pytest
and also provide the vscode config to run test on vscode easly, I don't know if that a great idea. You will decide. I also add to the module test cient class which work with sse for restframework and for django also I modified the event.py file to add function to the Event class. All older test are still working and I add some new test to test the new feature. Current coverage is 59%.
#TODO TEST CI #TODO IMPROVE THE DOCUMENTATION THAT NOT CLEAR in readme
TODO send_event() from object or data
Proposition :
-
Should we change all the file naming at the same time ? Because currectly thart very hard to read some file name: browsableapieventstreamrenderer.py browsable_api_event_stream_renderer.py -> maybe way better I already name the test file like this to make it more readable.
-
Should we use codecoverage and codecove.io ? With it we can display on the repo the current test coverage of the tests and get report of tests.