umap-project / umap

uMap lets you create maps with OpenStreetMap layers in a minute and embed them in your site.
https://umap-project.org
Other
1.2k stars 227 forks source link

Should we integrate with Django Channels? #2174

Open almet opened 1 month ago

almet commented 1 month ago

As part of our work on real-time collaboration, we're now shipping a WebSocket server as part of uMap. @yohanboniface suggested to take another look at Django Channels, hence this issue for tracking our thinking on the matter.

Pros:

Cons:

It's worth noting that some work has been done in the past to move to Django Channels.

yohanboniface commented 1 month ago

I'm in favor of going that way. I've revived your PR here: #2209

Here is a suggestion:

WDYT ? :)

almet commented 1 month ago

Let's go for a switch to Django Channels then :-) As mentioned in the chat, since we are reaching the end of the work for me on real-time collaboration, I think we should add support for Django Channels once all the rest is working, to avoid generating conflicts with the current design.

create an asgi.py file, alongside the current wsgi.py, so we can run uMap either with asgi or wsgi

Nice to be able to have the best of both worlds! That opens for a nice transition, I like it.

After #2209 is merged, here is a list of remaining things to do (but not right now if possible):

  • allow to run the sync websockets with channels
  • deploy ourselves with channels in our dev server and start testing
  • for next major release, document the asgi deployment, and warn about wsgi behing deprecated, but users can still deploy with wsgi (without the sync feature)
  • switch Docker image to asgi
  • next major release +1 (or +2), properly remove wsgi support

I like the way of offering a smooth transition. I have a minor change to propose: I don't believe we need to bump the major to offer the asgi support. It's a new feature, not a breaking change.

Also, I think we should offer at least 8 months for people to change their setup, between the first warning and the major, but I don't think this should block bumping the major. In other terms: if we release a new major in the height months, I would prefer to continue supporting wsgi in this release, and dropping it as soon as we reach the 8 month cap (the number of months can change obviously, this is an example).

yohanboniface commented 1 month ago

I don't believe we need to bump the major to offer the asgi support. It's a new feature, not a breaking change.

👍

Also, I think we should offer at least 8 months for people to change their setup

👍