Allow hostname RPC support#136
Conversation
|
👋 Thanks for assigning @benthecarman as a reviewer! |
Anyitechs
force-pushed
the
split-rpc-addr
branch
2 times, most recently
from
354137f to
0f7bc37
Compare
| ChainSource::Rpc { rpc_address, rpc_user, rpc_password } => { | ||
| builder.set_chain_source_bitcoind_rpc( | ||
| rpc_address.ip().to_string(), | ||
| rpc_address.port(), | ||
| rpc_user, | ||
| rpc_password, | ||
| ); | ||
| ChainSource::Rpc { rpc_host, rpc_port, rpc_user, rpc_password } => { | ||
| builder.set_chain_source_bitcoind_rpc(rpc_host, rpc_port, rpc_user, rpc_password); |
There was a problem hiding this comment.
Up to you people to make the call, I'd prefer parsing a single <hostname>:<port> string rather than splitting that setting into two parameters. This is what I do in VSS, it optimizes for fewer config params. That setting is logically a single thing right?
There was a problem hiding this comment.
That setting is logically a single thing right?
I agree it is. The problem was that we parse this setting using SocketAddr and that has a limitation of only supporting IP addresses. In containerized environments where the IP address might change if the container restarts, the user will have to update this setting each time it restarts. But supporting hostname simplifies things, because you only have to map the setting to a hostname and not worry if/when the IP changes.
An alternative to using SocketAddr (without splitting) is the ToSocketAddrs trait which supports hostname but it's sync and blocking.
UPDATE:
Missed "I'd prefer parsing a single <hostname>:<port>" from the comment earlier. Will drop the split.
Anyitechs
force-pushed
the
split-rpc-addr
branch
from
0f7bc37 to
a7278e5
Compare
tankyleo
requested review from
benthecarman and
tankyleo
and removed request for
jkczyz and
tankyleo
4 participants
This removes the restriction of
SocketAddrand allows us to support hostnames, which is essential in containerized/dynamic environments.Closes #66