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.

Sign up for GitHub

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

Jump to bottom

Improve get_ip_interface error message when interface does not exist #2964

Merged
merged 3 commits into from Aug 21, 2019
Merged

Improve get_ip_interface error message when interface does not exist #2964

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8f3a43a
Improve get_ip_interface error message.
lesteve Aug 19, 2019
ded57b4
black
lesteve Aug 19, 2019
c1015db
KeyError -> ValueError and reduce test dependence on message wording.
lesteve Aug 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 12 additions & 2 deletions distributed/tests/test_utils.py
View file
Open in desktop
Expand Up @@ -149,8 +149,18 @@ def test_get_ip_interface():
assert get_ip_interface("lo") == "127.0.0.1"
else:
pytest.skip("test needs to be enhanced for platform %r" % (sys.platform,))
with pytest.raises(KeyError):
get_ip_interface("__non-existent-interface")

non_existent_interface = "__non-existent-interface"
expected_error_message = (
"{!r} is not a valid network interface.+"
"Valid network interfaces are:".format(non_existent_interface)
)
if sys.platform == "darwin":
expected_error_message += ".+'lo0'"
elif sys.platform.startswith("linux"):
expected_error_message += ".+'lo'"
with pytest.raises(KeyError, match=expected_error_message):
Copy link
Member

Choose a reason for hiding this comment

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

Small nit:

Rather than test that the entire error message is exactly this, we might instead test some properties of it that we care about, like that "network interface" and the names of other interfaces are present in the message.

If someone changes the error message but includes this kind of information I think that that's ok, and shouldn't be considered an error.

get_ip_interface(non_existent_interface)


def test_truncate_exception():
Expand Down
11 changes: 10 additions & 1 deletion distributed/utils.py
View file
Open in desktop
Expand Up @@ -169,7 +169,16 @@ def get_ip_interface(ifname):
"""
import psutil

for info in psutil.net_if_addrs()[ifname]:
net_if_addrs = psutil.net_if_addrs()

if ifname not in net_if_addrs:
allowed_ifnames = list(net_if_addrs.keys())
raise KeyError(
"{!r} is not a valid network interface. "
"Valid network interfaces are: {}".format(ifname, allowed_ifnames)
)
Copy link
Member

Choose a reason for hiding this comment

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

This is great. I really like changes like this!


for info in net_if_addrs[ifname]:
if info.family == socket.AF_INET:
return info.address
raise ValueError("interface %r doesn't have an IPv4 address" % (ifname,))
Expand Down