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 · 7 comments · May be fixed by #419
Open

Feature: Adapter trait with optional async #5

Totodore opened this issue May 24, 2023 · 7 comments · May be fixed by #419
Assignees
Labels
A-redis-adapter Area related to redis-adapter A-socketioxide Area related to socketioxide C-Feature-request Request for a feature C-Refactoring Implies a refactoring I-Breaking-changes This issue implies breaking changes P-High High priority T-Docs Topic: documentation

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).

@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 this to Roadmap Oct 19, 2023
@Totodore Totodore moved this to Todo in Roadmap Oct 19, 2023
@Totodore Totodore added socketio-v4 A-socketioxide Area related to socketioxide labels Dec 15, 2023
@Totodore Totodore self-assigned this 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.

@torto
Copy link

torto commented Nov 19, 2024

Hello @Totodore I can work on that adpter for sure, I will start today have a look.

@Totodore Totodore linked a pull request Dec 21, 2024 that will close this issue
2 tasks
@Totodore Totodore removed a link to a pull request Dec 21, 2024
2 tasks
@Totodore Totodore added P-High High priority T-Docs Topic: documentation A-redis-adapter Area related to redis-adapter C-Refactoring Implies a refactoring C-Feature-request Request for a feature I-Breaking-changes This issue implies breaking changes labels Jan 1, 2025
@Totodore Totodore moved this from Todo to In Progress in Roadmap Jan 1, 2025
@Totodore Totodore linked a pull request Jan 1, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-redis-adapter Area related to redis-adapter A-socketioxide Area related to socketioxide C-Feature-request Request for a feature C-Refactoring Implies a refactoring I-Breaking-changes This issue implies breaking changes P-High High priority T-Docs Topic: documentation
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

5 participants