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

feat(cli): automatically determine rpc host/port #1462

Merged
1 commit merged into from
Apr 15, 2020
Merged

Conversation

sangaman
Copy link
Collaborator

@sangaman sangaman commented Apr 8, 2020

This enhances the cli to automatically attempt to find and read from the xud configuration file to determine the port and host for the xud gRPC service based on the configured network - which determines default port numbers - or configured port if specified. This prevents the user from having to specify the rpc port on xucli commands when using a non-mainnet xud instance or a non-default gRPC port via the config file.

Closes #1451.

@sangaman sangaman added the command line (CLI) Relating to the command line interface tools label Apr 8, 2020
@sangaman sangaman self-assigned this Apr 8, 2020
@sangaman sangaman requested review from a user and kilrau April 8, 2020 16:48
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

The feature itself works nicely for all networks 👍

The only minor nit I have is the error message when xud is not running:

➜  xud git:(cli/detect-network) ✗ ./bin/xucli getinfo -j | jq
could not connect to xud at undefined:undefined, is xud running?

Copy link
Contributor

@kilrau kilrau left a comment

Choose a reason for hiding this comment

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

LGTM 💯

BEWARE @sangaman : needs xud-docker to remove applying a xud.conf which sets gRPC port to 8886 for all networks, so please coordinate with @reliveyy

@sangaman
Copy link
Collaborator Author

The only minor nit I have is the error message when xud is not running:

➜  xud git:(cli/detect-network) ✗ ./bin/xucli getinfo -j | jq
could not connect to xud at undefined:undefined, is xud running?

Good catch, I put in a quick fix to preserve the port and host we determined to use. Thanks.

@ghost
Copy link

ghost commented Apr 14, 2020

I'm now getting:

➜  xud git:(cli/detect-network) ✗ ./bin/xucli getinfo
Error: 14 UNAVAILABLE: DNS resolution failed

using this branch (when xud is running & also closed).

argv.rpcport = config.rpc.port;
argv.rpchost = config.rpc.host;

return new XudClient(`${argv.rpchost}}:${argv.rpcport}`, credentials);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like I typoed an extra } here after rpchost which is why the DNS resolution failures were happening, it was trying to resolve localhost} instead of localhost. Took me a while to figure out what was going on because I could've sworn it was working for me when I had tested it then I was getting the dns error too out of the blue. Thanks for catching this @erkarl .

This enhances the cli to automatically attempt to find and read from
the xud configuration file to determine the port and host for the xud
gRPC service based on the configured network - which determines default
port numbers - or configured port if specified. This prevents the user
from having to specify the rpc port on `xucli` commands when using a
non-mainnet xud instance or a non-default gRPC port via the config file.

Closes #1451.
@ghost ghost merged commit ce0824e into master Apr 15, 2020
@ghost ghost deleted the cli/detect-network branch April 15, 2020 11:08
sangaman added a commit that referenced this pull request Apr 21, 2020
This resolves a merge issue between #1462 and #1481 where the
`loadXudClient` method was made async in the latter but still used like
a synchronous call in the former.
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command line (CLI) Relating to the command line interface tools P2 mid priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Let xucli automatically determine gRPC port
2 participants