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

Feature: Adapter trait with optional async #5

Open
Totodore opened this issue May 24, 2023 · 6 comments
Open

Feature: Adapter trait with optional async #5

Totodore opened this issue May 24, 2023 · 6 comments
Assignees
Labels
enhancement New feature or request refactoring This reference a need for a refactoring socketio-v4 It is only about the version 4 of the socket.io protocol socketio-v5 It is only about the version 5 of the socket.io protocol

Comments

@Totodore
Copy link
Owner

Make a version of the adapter trait async. This will allow remote adapter implementation (e.g redis adapter, mongodb adapter).

@Totodore Totodore added the enhancement New feature or request label May 24, 2023
@Shorakie
Copy link

does the maybe_async crate cut it?

@Totodore
Copy link
Owner Author

Yes, it is a good idea, but it implies to also apply maybe_async on Operators and Socket structs to maybe return futures from the adapter.

@Totodore Totodore added help wanted Extra attention is needed refactoring This reference a need for a refactoring labels Nov 23, 2023
@Totodore Totodore added socketio-v4 It is only about the version 4 of the socket.io protocol socketio-v5 It is only about the version 5 of the socket.io protocol labels Dec 15, 2023
@Totodore Totodore self-assigned this Dec 20, 2023
@Totodore Totodore removed the help wanted Extra attention is needed label Dec 20, 2023
@ElijahJohnson5
Copy link

This would be awesome to have! Really enjoy the work you are doing on this

@headironc
Copy link
Contributor

I think there may be scenes that require both async and sync.

@Totodore
Copy link
Owner Author

Sure, the compatibility with previous sync code and the possibility to use both are my main blocking points to move on on this feature.
Currently one of the approaches I considered is using Operators to express multi-node async code and keep sync operations on emit/room/ns by default.

socket.emit("test", ()); // default sync behavior that will emit only from this node.
socket.async().emit("test", ()).await; // async behavior shared between nodes.

Other possibilities are:

  • to use feature flags to transform all sync function to async when enabling the async feature on but it would break the default sync feature and features should always be additive for these reasons.
  • To remove sync possibilities on emit/rooms/ns functions and to have everything async by default.

@Totodore
Copy link
Owner Author

Totodore commented Nov 5, 2024

The PR #395 solves the async adapter issues. However before merging this to the main branch I want to be sure that we have a stable trait for all the adapter implementations.

Therefore the PR #395 will be merged on the feat-adapter-rework branch. And the adapter implementions should be worked on this branch. I would like to have at least 2 implementations (maybe redis and mongo before merging everything to main and making a release).

Currently the Adapter trait is in the socketioxide crate, but I would like to put it in the socketioxide-core crate so that custom adapter implementations only depends on the core crate. Also the implementations should live in this repository in the crates folder.

@torto once PR #395 is merged, would you still like to work on a Mongo adapter (See issue #361)?. I will personnally try to work on a Redis adapter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactoring This reference a need for a refactoring socketio-v4 It is only about the version 4 of the socket.io protocol socketio-v5 It is only about the version 5 of the socket.io protocol
Projects
Status: Todo
Development

No branches or pull requests

4 participants