Skip to content
This repository has been archived by the owner on Feb 29, 2024. It is now read-only.

support setting ZMQ socks proxy #2400

Merged

Conversation

vitalrev
Copy link
Contributor

implementation for issue #2399

Signed-off-by: Vitalij Reicherdt [email protected]

Signed-off-by: Vitalij Reicherdt <[email protected]>
Copy link
Contributor

@dhh1128 dhh1128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution to a pesky problem! Thank you!

@vitalrev
Copy link
Contributor Author

Thanks!
As first-time contributor, i can not merge this pull request by my self.
Waiting for approval and merge from a maintainer with write access...

@dhh1128
Copy link
Contributor

dhh1128 commented Jun 22, 2021

Although I have rights to merge, I am not as close to this code as I used to be. I suggest that we get @jovfer to review.

Sergey, this PR looks very straightforward; <1 min for you to review. If you give your approval, I will merge it?

@jovfer
Copy link
Contributor

jovfer commented Jun 23, 2021

@vitalrev Please take a look on API params related to pool open operation https://github.com/hyperledger/indy-sdk/blob/master/libindy/src/api/pool.rs#L68-L82
Should this ZMQ setting be a part of API params or do you suggest to ignore api capability and use env variable for the setting?

@dhh1128 thanks for the heads up.

PS: initially the config was used for different kind of settings, so this is rather a question, not a change request

@vitalrev
Copy link
Contributor Author

@jovfer thank you for support!
what you mean is the pool configuration here and Socket creation here
This is an inproc://{} socket for ØMQ local in-process (inter-thread) communication transport.

But we need the socks proxy configuration only for the communication to the remote nodes via tcp://... and this is implemented in networker.rs RemoteNode.connect.

@jovfer
Copy link
Contributor

jovfer commented Jun 25, 2021

@vitalrev sorry, didn't get your point.

The pool config from API goes down to the service as you linked
but then it's forwarded not to internal sockets, but to Pool::new(...) here. Finally the config structure is split to separate values and some of them is available in RemotNode.connect.

@vitalrev
Copy link
Contributor Author

vitalrev commented Jul 9, 2021

@jovfer ok, I'm trying to include the socks_proxy config in the PoolOpenConfig and pass it on to RemoteNode.

… the socks_proxy - to set ZMQ SOCKS_PROXY parameter

Signed-off-by: Vitalij Reicherdt <[email protected]>
@vitalrev
Copy link
Contributor Author

@jovfer I rebuilt it ... it became a little more than 2 lines of code. I hope it goes with your suggestion.
Please review

@dhh1128
Copy link
Contributor

dhh1128 commented Jul 13, 2021

I am going to merge this, now that @jovfer has approved. Just waiting on builds to pass.

@vitalrev
Copy link
Contributor Author

vitalrev commented Jul 13, 2021

I am going to merge this, now that @jovfer has approved.

thank you!

@dhh1128 dhh1128 merged commit 113b79c into hyperledger-archives:master Jul 13, 2021
@vitalrev vitalrev deleted the feature/support-zmq-socks-proxy branch July 13, 2021 10:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants