-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add room storage for Django Channels Consumers #25
base: main
Are you sure you want to change the base?
Add room storage for Django Channels Consumers #25
Conversation
Thanks for submitting your first pull request! You are awesome! 🤗 |
Thanks @cacosandon for the PR. |
Hey @davidbrochart! Sorry for the delay. The core distinction between On the other hand, Django Channels uses a decentralized model, where each consumer operates in an independent execution thread. Our approach offers an "ephemeral" storage solution, keeping everyone in sync without manually managing each document's store lifecycle. In summary, both systems aim to synchronize and persist document states. However, the decentralized nature of Django Channels requires a Let me know what u think! |
Hey @davidbrochart, have you taken a look at this? 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay @cacosandon.
I'm not sure about these changes, I still think that YRoomStorage is similar to YStore.
@@ -0,0 +1,3 @@ | |||
from .yjs_consumer import YjsConsumer as YjsConsumer | |||
from .yroom_storage import BaseYRoomStorage as BaseYRoomStorage | |||
from .yroom_storage import RedisYRoomStorage as RedisYRoomStorage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that redis is a required dependency when using Django channels?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right. I've removed RedisYRoomStorage
from this init file and separate them in files so when you import the base class you are not requiring redis. Maybe we should do the same with sqlite-anyio
on YStore, which is a dependency for the whole project.
…s its not required and update imports
…torage and close method to base style: fix linter wrong return type of save_snapshot
f4c2b06
to
f7c6f26
Compare
Thank you, @davidbrochart, for the comments! I think the primary distinction lies in our need for a shared While they are very similar—implementing the same methods for applying updates and writing—what they lack is an "ephemeral" storage to manage each of the updates from clients (or consumers in Django Channels). Additionally, we don't require the property Thus, while we could use Let me know what you think, and thanks again :) |
… Redis and make make_redis public
There could be a new in-memory implementation of a YStore.
Usually services have to do some kind of initialization and teardown, but if you don't need that then nothing prevents you from doing nothing in these methods.
But that's also a maintenance effort that I'd rather not do. |
This PR introduces a shared storage for consumers within Django Channels.
Initially, each YjsConsumer had its unique YDoc, updating only from its client. This approach caused issues, such as consumers having different YDoc versions that made difficult to update to a persistent storage.
Now, by inheriting from BaseYRoomStorage, a unified storage can be utilized across all consumers and server-wide (e.g., in a Django Celery Job).
The storage's function, detailed in the documentation, is to manage document changes—fetching, updating, and saving them to a single source. An implementation example features Redis for ephemeral and Postgres for permanent storage.
Each user can define their own storage. Maybe using Django Cache, custom Redis or no storage at all (and just use it to broadcast updates from their connected clients). We just define the interface.