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

Fix for #925, added support for tilda in provider ipc path #1049

Merged
merged 4 commits into from
Apr 25, 2019

Conversation

pjryan93
Copy link

What was wrong?

Tilda's are not being replaced with the users home directory on Linux and Mac.

w3 = Web3(Web3.IPCProvider("/Users/pjryan/.ethereum/testnet/geth.ipc")); 
w3.isConnected()  #returns True

w3 = Web3(Web3.IPCProvider("~/.ethereum/testnet/geth.ipc")); 
w3.isConnected() #returns False

Related to Issue #
Fix for #925.

How was it fixed?

Call expanduser() and resolve() to make sure the Tilda is replaced with the user's home directory.

Cute Animal Picture

happy cat!

@carver
Copy link
Collaborator

carver commented Sep 10, 2018

Thanks!

Interesting: the new resolve() call is causing tests to fail when the path to the IPC file doesn't exist (or has restricted permissions). I think this is what we want, and the tests should be updated to use viable paths, like:

-('file:///root/path/to/file.ipc', IPCProvider, {'ipc_path': '/root/path/to/file.ipc'}),
+('file:///tmp/file.ipc', IPCProvider, {'ipc_path': '/tmp/file.ipc'}),

(Or presumably using a method to generate /tmp path instead, so it works across platforms.)

If this is unclear, or you get stuck please reach out and ask for help.

@carver
Copy link
Collaborator

carver commented Sep 24, 2018

Actually, since this is only breaking in python 3.5, this PR can just wait till 3.5 is dropped in web3.py v5 and merge then. I'm doing a backport to web3 v4 currently.

@carver
Copy link
Collaborator

carver commented Apr 25, 2019

You're going to rebase right @kclowes ? :)

@kclowes kclowes merged commit 71b8bf0 into ethereum:master Apr 25, 2019
@kclowes
Copy link
Collaborator

kclowes commented Apr 25, 2019

Thanks @pjryan93!

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.

3 participants