Skip to content
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 #94

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

cacosandon
Copy link

@cacosandon cacosandon commented Mar 27, 2024

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.

I would like to add some tests too 🤔 but not sure how.

cacosandon and others added 21 commits March 21, 2024 16:11
…ss for clarity and consistency

docs(yroom_storage.py): clarify comments regarding YRoomStorage instances and document updates for Django Channels Consumers
Add room storage for Django Channels Consumers
@zswaff
Copy link
Contributor

zswaff commented Mar 28, 2024

@davidbrochart FYI: @cacosandon has been working on this as a big upgrade to #84 that I worked on a while ago. It is a significant improvement to usability and functionality of using all of this with Django/Channels in my opinion.

It does introduce some breaking changes to the Channels interface (moved the class to a folder, removed one method), but nothing too serious. I already reviewed the work and it works well for our case at Dart.

Would love to support getting this merged however possible, just let me/us know!

@davidbrochart
Copy link
Collaborator

Thanks @cacosandon and @zswaff for working on this.
Unfortunately ypy is unmaintained, and so is ypy-websocket. I started pycrdt and pycrdt-websocket that are meant to replace them. pycrdt-websocket has the Django Channels Consumer so I think you could make an equivalent PR there.
What do you think?

@zswaff
Copy link
Contributor

zswaff commented Mar 29, 2024

Makes sense to me! I think this PR will be just as good for that repo.

I didn't know about the change. Are you planning to archive these repos and update the docs to point people to the new ones?

@davidbrochart
Copy link
Collaborator

I don't think so, ypy is looking for maintainers.

@cacosandon
Copy link
Author

Thanks @zswaff! @davidbrochart, I've just created this same PR in your repository following the library changes.

It works perfect too! jupyter-server/pycrdt-websocket#25

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants