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 vitalij.reicherdt@commerzbank.com

Signed-off-by: Vitalij Reicherdt <vitalij.reicherdt@commerzbank.com>
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 <vitalij.reicherdt@commerzbank.com>
@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.

None yet

3 participants