housleyjk / ws-rs

Lightweight, event-driven WebSockets for Rust.
MIT License
1.46k stars 219 forks source link

add unix domain socket support for server side #342

Closed Young-Flash closed 1 year ago

Young-Flash commented 1 year ago

Hi there. I add unix domain socket support for server side. Current api requires TcpStream and SocketAddr strictly, I wrap them into enum and modified other method according the actual type, most of the changes are in src/io.rs. Demo can be found at example/unix_server.rs. Welcome to share your ideas If you think there are other way to implement this feature. @housleyjk @kevincox

60

housleyjk commented 1 year ago

If you have time, can you provide a reason for using the deprecated parts of mio to accomplish this?

I totally understand that bumping the mio version and refactoring may not be worthwhile, but that raises the question of whether it is worth doing a release with these changes if we don't have the bandwidth to upgrade. Basically, this seems like a new feature, which is great, but if we are doing new features, it would be nice to be using more recent dependencies, etc. The PR is not a simple bug fix, so I am hesitant to merge and release it where we don't have more development resources to back it up.

Young-Flash commented 1 year ago

Actually I have thought about update the mio dependency so as to use mio::net::UnixStream, however, you know, there are various major changes in new version mio, and seems mio-extras will no update to new version mio according to this. So I have to use mio::deprecated::unix::UnixStream to implement this feature for now. Do you have plan to update mio to latest version or remove mio-extras? Seems it require a lot of work. IMO, If ws-rs will be continuously maintained in the future, it's necessary to update the dependency. What do you think?

housleyjk commented 1 year ago

I agree. Right now I am not working on new features, and I don't think I have time right now to do a significant refactor to support more recent mio. That is why I am thinking it would be better to only merge bug fixes to existing features rather than add new features.

Young-Flash commented 1 year ago

well. I could help refactor to update to new version mio if I have time. Do you have any ideas about the refactor? It seems mio-extras will not be maintained so maybe we should remove it.

housleyjk commented 1 year ago

You are welcome to refactor, but I don't have the time to encourage you right now. Please only do it if you want to, and you are free to make dependency changes that you think are worthwhile, but do not remove any existing features. If you think something needs to be deprecated, that's ok, but if refactoring requires breakage, then that would be the time where we could release with deprecated parts of dependencies.

Thank you for your interest!

Young-Flash commented 1 year ago

I tried to remove mio-extras and upgrade mio these days and I found it's tough, channel and timer are tightly coupled in the project, they depend on Registration and SetReadiness, which was remove from new version mio. I need unix socket feature and I would appreciate if you merge this PR, there are not many changes and I could help to refactor it if mio is updated in the future. After all, we can't ignore new features all the time because of uncertain update time.

housleyjk commented 1 year ago

I'd rather not merge these changes if they are unlikely to be in a release. I support you forking the project for your own needs.