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

Does not work on Windows #5

Closed
asantoni opened this issue Apr 16, 2018 · 5 comments
Closed

Does not work on Windows #5

asantoni opened this issue Apr 16, 2018 · 5 comments

Comments

@asantoni
Copy link
Contributor

aioping does not work correctly on Windows 10 because this code around line 230 of __init__.py blows up:

    loop = asyncio.get_event_loop()
    info = await loop.getaddrinfo(dest_addr, 0)
    family = info[2][0]
    addr = info[2][4]

You get an exception because info is only of length 1 on Windows.

This code looks sketchy to me (why choose the third info element?), so I hacked it to just pick a random element and I'll send a PR shortly. My understand is that this is doing a DNS lookup, and if there's multiple records returned, we should picking a random one. The code should probably be extended to handle IPv6 more explicitly too, but I'll leave that as an exercise for someone else!

asantoni added a commit to asantoni/aioping that referenced this issue Apr 16, 2018
* If multiple IPs are resolved from a hostname, randomly choose one of
those IPs instead of hardcoding it to use the third one.
@anton-belousov
Copy link
Contributor

@asantoni thanks for the noticing that and for the PR, I hope will have time to review it and update the library by the end of the week (have a huge workload currently)

@bradh11
Copy link

bradh11 commented Jun 15, 2018

is this still an issue?

@asantoni
Copy link
Contributor Author

Yes, looks like only one new (unrelated) commit in master since I opened this.

If you need it working on Windows right now, feel free to use my fork temporarily. (asantoni/aioping)

anton-belousov added a commit that referenced this issue Jun 15, 2018
Fix issue #5: Does not work on Windows
@anton-belousov
Copy link
Contributor

Sorry for delay guys, just merged the request, will update the distro now. @asantoni thanks a lot for the fix!

@bradh11
Copy link

bradh11 commented Jun 15, 2018

confirmed working/fixed. Thanks for the merge! great library btw.

Crypto-Spartan pushed a commit to Crypto-Spartan/aioping that referenced this issue Oct 30, 2020
* If multiple IPs are resolved from a hostname, randomly choose one of
those IPs instead of hardcoding it to use the third one.
Crypto-Spartan pushed a commit to Crypto-Spartan/aioping that referenced this issue Oct 30, 2020
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

No branches or pull requests

3 participants