Add default trinity locations to IPC path guesser #1121
Conversation
web3/providers/ipc.py
Outdated
| @@ -91,6 +93,14 @@ def get_default_ipc_path(testnet=False): | |||
| if os.path.exists(ipc_path): | |||
| return ipc_path | |||
|
|
|||
| base_trinity_path = Path('~').expanduser() / 'Library' / 'Application Support' / 'trinity' | |||
| if testnet: | |||
| ipc_path = base_trinity_path / 'ropsten' / 'jsonrpc.ipc' | |||
There was a problem hiding this comment.
This seems problematic. What happens when trinity adds supports for other networks like rinkeby?. A boolean testnet won't be sufficient to guess the path.
There was a problem hiding this comment.
You can, of course, try all possible paths and check if the path exists?
But what happens when a machine is running both ropsten and rinkeby? :D
It will always pick the first path it comes across!
There was a problem hiding this comment.
Yeah, that's problematic. Wondering if we should do something like:
- mainnet remains special (and we can also implement some checks to validate that we are indeed connected to the mainnet
- use a name-based pattern for all the other networks (
rinkeby/kovan/ropsten) and somehow namespace the imports asfrom web3.auto.ropsten import ...
There was a problem hiding this comment.
Yup, I am pretty happy with the idea of dropping support for implicitly detecting the testnet IPC (where from web3.auto.ropsten import w3 is the next-best option for users).
web3/providers/ipc.py
Outdated
| @@ -91,6 +93,14 @@ def get_default_ipc_path(testnet=False): | |||
| if os.path.exists(ipc_path): | |||
| return ipc_path | |||
|
|
|||
| base_trinity_path = Path('~').expanduser() / 'Library' / 'Application Support' / 'trinity' | |||
There was a problem hiding this comment.
Is this really where XDG directories end up for osx users?
There was a problem hiding this comment.
Not sure, I was cribbing off of Parity since I don't have a Mac to test with.
According to this stackoverflow post it might actually be ~/Library/trinity instead.
We might want to use a more unique namespace on the trinity side, actually, like ~/Library/org.ethereum.trinity. For Mac, Linux, everything.
There was a problem hiding this comment.
@carver Below is the path on my mac machine:
/Users/voith/.local/share/trinity/mainnet/jsonrpc.ipc
There was a problem hiding this comment.
However for parity, the path is:
/Users/voith/Library/Application Support/io.parity.ethereum/jsonrpc.ipc
There was a problem hiding this comment.
Weird, I think I might call that a trinity bug. But I don't know Mac well enough to say whether the Mac convention should override the ~/.local/share default described by https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html for XDG_DATA_HOME.
There was a problem hiding this comment.
@carver I'm ok considering the move, but only in the face of an objective reason to do so. I really like the simplicity of XDG and it seems like an easier to find location for most developers.
There was a problem hiding this comment.
but only in the face of an objective reason to do so
Fair enough, and I don't think I can speak for our Mac users preferences here, so I'll drop it for now. I'll make the change in this PR to look in the ~/.local location on Mac.
carver
force-pushed
the
trinity-auto-ipc
branch
from
29aceb0
to
e013feb
Compare
What was wrong?
web3py wasn't auto-detecting trinity path
How was it fixed?
Added the trinity paths for mac & linux.
Cute Animal Picture