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

get_room should not create a room, since YRoom might be subclassed #34

Open
Zsailer opened this issue Apr 26, 2024 · 2 comments
Open

get_room should not create a room, since YRoom might be subclassed #34

Zsailer opened this issue Apr 26, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@Zsailer
Copy link
Member

Zsailer commented Apr 26, 2024

Right now, get_room will create an instance of a YRoom if the room doesn't exist. This is problematic for libraries that subclass YRoom, e.g. jupyter-collaboration uses it's own DocumentRoom subclass. The room is created and added to the Websocket Server using the add_room API. This can lead to an unintentional creation of a mix of YRoom and DocumentRoom instances.

Copy link

welcome bot commented Apr 26, 2024

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@Zsailer Zsailer added the bug Something isn't working label Apr 26, 2024
@davidbrochart
Copy link
Collaborator

Thanks @Zsailer.
The get_room() method in pycrdt-websocket is used to serve a WebSocket client. If the room doesn't exist yet, one must be created. A subclass of WebsocketServer must ensure its custom room is created before serving the WebSocket. But in the general case where WebsocketServer is not subclassed, we need to have a default way of creating a room.
But I understand your point, and maybe we should have a create_room() in pycrdt-websocket, that would create a generic room by default, that WebsocketServer subclasses could override with their custom implementation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants