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 missing zmq_proxy method #127

Closed
xpepermint opened this issue Jan 24, 2021 · 5 comments
Closed

Add missing zmq_proxy method #127

xpepermint opened this issue Jan 24, 2021 · 5 comments

Comments

@xpepermint
Copy link

Hey, I noticed there is no Proxy component available:

zmq_proxy (frontend, backend, capture);
@Alexei-Kornienko
Copy link
Collaborator

Originally I planned to add such a method. However currently I'm considering first implemeting this feature #112 . If we'll provide basic Stream/Sink implementation we could use https://docs.rs/futures/0.3.12/futures/stream/trait.StreamExt.html#method.forward method. I'll try to prioritize this features if they are important to you

@xpepermint
Copy link
Author

Thanks, mate!

@Alexei-Kornienko
Copy link
Collaborator

I've added a prototype implementation of proxy function.

It looks like that using native StreamExt::forward doesn't cover case with duplex messaging (frontend <--> backend). Cause it looks like that it's only suitable for case frontend --> backend

Cause of this I've added a generic function with API similar to the one that zmq provides.
One of the drawbacks of such impl is that it requires both sockets to implement both SocketSend and SocketRecv with is not the case for some sockets (pub for example only implements SocketSend).
For such cases I think we can still use stream.forward

@xpepermint , @TheButlah What do you think?

@xpepermint
Copy link
Author

Thanks, @Alexei-Kornienko for the PR. I appreciate your dedication and fast response. Yes, the proxy should work both ways and you won't be able to do this using the Stream object. The loop with select looks good to me, the interface supports all the required features so I guess this is it. You could just also add a test case for this new feature so we have it all covered.

@Alexei-Kornienko
Copy link
Collaborator

Consider to be closed for now. May come back to it later in case of any issues.

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

No branches or pull requests

2 participants