-
Notifications
You must be signed in to change notification settings - Fork 161
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
Replace unmaintained netifaces
library to avoid local wheel builds
#875
Conversation
Signed-off-by: Laurenz Altenmüller <laurenz.altenmueller@fernride.com>
1cc9736
to
c756385
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So 2 years ago, in #687, I switched ros2doctor
from using python3-ifcfg
to using python3-psutil
for network configuration. If it is possible, I think we should prefer to switch to using python3-psutil
here to reduce our dependencies. @lalten can you take a look at the python3-psutil
APIs and see if we can accomplish the same thing?
Needed in ros2/ros2cli#875
Oh very cool, I had no idea psutil could do network things. I will give it a try. |
Signed-off-by: Laurenz Altenmüller <laurenz.altenmueller@fernride.com>
I changed to psutil, it works great and is very compatible to previous behavior. Would be happy about a review @clalancette :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks. I'm happy to get rid of one additional dependency. I'll run full CI on this next.
@lalten thanks for the contribution. |
I'd like to keep this in Rolling for a while before we do that, to see if any problems come up. Even so, I actually don't see a compelling reason to do it. |
netifaces is unmaintained: al45tair/netifaces#78.
It does not provide official prebuilt wheels for Python > 3.9: https://pypi.org/project/netifaces/#files
https://pypi.org/project/psutil/ is an alternative (available in rosdep as
python3-psutil
) but the interface is a little different.The reason I'm making this change is that we want to upgrade to a Python>3.9 but not provide a compiler inside our CI container (Unrelated to this PR, but we're using Bazel with a standalone hermetic compiler toolchain and https://github.com/mvukov/rules_ros2). Not having a host compiler for Python wheel builds makes it more difficult to build netifaces.
There are only two
netifaces
usages in ros2cli:LocalXMLRPCServer
It just needs a list of available IPv4s (only
AF_INET
, noAF_INET6
). ifaddr can provide a one-line replacement for theget_local_ipaddrs()
function:NetworkAwareNode
It must restart if network interfaces change. Currently it's checking for changes by comparing a (default)dict containing netiface's data.
In my ifaddr replacement it will instead compare a set of repr-strings that contain all data ifaddr provides.
Note that I haven't tested this, but this should work fine on Windows as well.