SamuelYvon / netifaces-2

netifaces reborn
https://pypi.org/project/netifaces2/
MIT License
21 stars 7 forks source link

Windows fixes and iface indexes #29

Closed relativityspace-jsmith closed 4 months ago

relativityspace-jsmith commented 4 months ago

Hello! After what we talked about last week, I prototyped a way to add interface indices to the netifaces2 API. Along the way, I also found some bugs on Windows and am working through fixing them.

Changes made so far:

SamuelYvon commented 4 months ago

I could NOT resist renaming some of the linux_xxx functions to posix_xxx ones, because I'm kinda triggered by the fact that they say linux but work for mac as well

I like that thinking ;) Gotta make sure it's fully posix-compliant however. I'll setup a more complete test setup

relativityspace-jsmith commented 4 months ago

Thanks for the feedback! Any way you could approve my workflow run so I can see how badly I broke the CI?

relativityspace-jsmith commented 4 months ago

Also, re the Windows memory issue, I do not see the issue on my machine and will keep an eye out for any machines that do have it!

relativityspace-jsmith commented 4 months ago

Also, should mention, there is still one serious issue with netifaces-2 that would keep me from using it with multicast_expert: currently, loopback interfaces are not returned by netifaces.interfaces() on Windows (though they are on other platforms). I suspect this is related to using GetAdaptersInfo() in this library verseus netifaces using GetAdaptersAddresses(), because the original netifaces is able to find the loopback interface somehow.

Will have to look into this in a subsequent MR.

SamuelYvon commented 4 months ago

Also, re the Windows memory issue, I do not see the issue on my machine and will keep an eye out for any machines that do have it!

I was never able to reproducer either :/ the code looks right also, but apparently is not. See issue https://github.com/SamuelYvon/netifaces-2/issues/21

loopback interfaces are not returned Will have to look into this in a subsequent MR.

I would greatly appreciate! If you don't have the time I can also take a look this weekend but I'm not sure how much time I actually have to put towards this

relativityspace-jsmith commented 4 months ago

Workflow approve again? It looks like it needs approval each time X_X

relativityspace-jsmith commented 4 months ago

Also, side note, I'm having an issue that's really annoying. It seems like ruff is enforcing a line length of <=88 characters, but black is not configured to reformat down to that line length. So, running black doesn't make my line lengths correct and I have to enforce that manually. Any idea how to fix this? I tried setting the black line length in pyproject.toml but that didn't fix it.

P.S. I would be really happy if we could increase the line length to be a bit more generous, like 120 chars. 88 chars feels like I'm being squeezed into a tiny box...

SamuelYvon commented 4 months ago

Yep, I agree. I'll fix that ASAP

SamuelYvon commented 4 months ago

If you rebase on dev you'll have the changes.

relativityspace-jsmith commented 4 months ago
    hooks:
      - id: black
        args: [--config=./pyproject.toml]

ohhh, thaaaaat's what I was missing 🤦‍♂️

SamuelYvon commented 4 months ago

Workflow approve again? It looks like it needs approval each time X_X

Should be more lenient now

relativityspace-jsmith commented 4 months ago

OK I think this is ready for merge!

SamuelYvon commented 4 months ago

Thanks for the hard work!