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

[INDY-2330] Tool for checking zmq connection #1471

Merged
merged 5 commits into from
Feb 17, 2020

Conversation

lampkin-diet
Copy link

Signed-off-by: Andrew Nikitin andrew.nikitin@dsr-corporation.com

Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Andrew Nikitin added 2 commits February 13, 2020 16:00
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
packages=find_packages(),
package_data={'': ['*.md']},
include_package_data=True,
install_requires=['libnacl==1.6.1', 'base58==1.0.0'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to install libnacl explicitly? Do we need both python3-nacl (in the Docker file) and libnacl?

Copy link
Author

Choose a reason for hiding this comment

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

I think, that Dockerfile needs only for demonstration purposes. But in general, maybe python3-nacl is enough.

parser.add_argument('--tcp_port', help="Port which will be used for TCP client's connections")
args = parser.parse_args()

zmq_server_ha = HA('0.0.0.0', int(args.zmq_port) if args.zmq_port else '9999')
Copy link
Contributor

Choose a reason for hiding this comment

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

We may not always bind to 0.0.0.0 in production cases

Copy link
Author

Choose a reason for hiding this comment

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

Agree, I will add option for choosing bind address and keep '0.0.0.0' as default.

s.bind(server_ha)
s.listen()
print("TCP_SERVER: Listen clients on {}".format(server_ha))
while True and not QUIT:
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like QUIT is always False, isn't it?

Copy link
Author

Choose a reason for hiding this comment

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

I forgot to remove it. It was some experiments need for demo.

Andrew Nikitin added 2 commits February 14, 2020 19:26
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>
Signed-off-by: Andrew Nikitin <andrew.nikitin@dsr-corporation.com>

# Client
As a client this tool has the next required input parameters:
``
Copy link
Contributor

Choose a reason for hiding this comment

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

Please user ``` instead of ``

``
`--zmq` and `--tcp` options are required, because they need to determine address/port tuple for remote connection for tcp and zmq protocols.
Also, it's important to make sure that both of protocols are acceptable to create connection between two nodes.
`address` - it's IP address of node which will be used to run this script as `server`.
Copy link
Contributor

Choose a reason for hiding this comment

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

What about port?


# Example
Let client's machine will be `10.0.0.1` and server's `10.0.0.2` IP addresses. In this case, for checking connections we can run server as:
``
Copy link
Contributor

Choose a reason for hiding this comment

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

Please user ``` instead of ``

`--addr` parameter is optional and define IP address for binding. '0.0.0.0' will be used by default.

# Example
Let client's machine will be `10.0.0.1` and server's `10.0.0.2` IP addresses. In this case, for checking connections we can run server as:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Let client's machine will be `10.0.0.1` and server's `10.0.0.2` IP addresses. In this case, for checking connections we can run server as:
Let client's machine IP address be `10.0.0.1` and server's machine IP address be `10.0.0.2`. In this case, for checking connections we can run the server as:


if [ ! -d $V_DIR/$V_NAME ]; then
virtualenv $V_NAME
$V_DIR/$V_NAME/bin/pip install /tmp/test_zmq/dist/plenum-zmq-check-1.0.0.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

Why /tmp/test_zmq/dist/plenum-zmq-check-1.0.0.tar.gz?

check_zmq.sh client '--zmq 10.0.0.2:9999 --tcp 10.0.0.2:10000'
``


Copy link
Contributor

Choose a reason for hiding this comment

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

Please mention what this script will do so that's this is not a surprise that it installs virtualenv.

Copy link
Author

Choose a reason for hiding this comment

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

Ok. I'll add some comments regarding requirements for installation too, like python3, pip3, etc...

Copy link
Contributor

@ashcherbakov ashcherbakov left a comment

Choose a reason for hiding this comment

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

In general looks good.
There are some comments applying to docs, so it can be processed in another PR.

@ashcherbakov ashcherbakov merged commit c4cdf03 into hyperledger:master Feb 17, 2020
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

Successfully merging this pull request may close these issues.

None yet

2 participants