Skip to content

Fix typo in create_datagram_endpoint#46

Merged
1st1 merged 1 commit intoMagicStack:masterfrom
vodik:master
Aug 24, 2016
Merged

Fix typo in create_datagram_endpoint#46
1st1 merged 1 commit intoMagicStack:masterfrom
vodik:master

Conversation

@vodik
Copy link
Copy Markdown
Contributor

@vodik vodik commented Aug 13, 2016

Typo causes create_datagram_endpoint to send traffic to the local_addr instead of the remote_addr.

Closes #45.

@1st1
Copy link
Copy Markdown
Member

1st1 commented Aug 13, 2016

Hi! Thank you for this PR! Would you be able to write a regression test?

@vodik
Copy link
Copy Markdown
Contributor Author

vodik commented Aug 13, 2016

I'll see what I can do with different loopback address.

@vodik
Copy link
Copy Markdown
Contributor Author

vodik commented Aug 13, 2016

Okay, I've modified the UDP test case to set a fixed local_addr of 127.0.0.2. This causes the UDP tests to fail on master, but work with this patch.

If you rather a separate test case, I can do that too, but mind giving me some pointers how I should organise things?

@1st1
Copy link
Copy Markdown
Member

1st1 commented Aug 16, 2016

@vodik With this PR test_create_datagram_endpoint_addrs fails on MacOS X:

======================================================================
ERROR: test_create_datagram_endpoint_addrs (test_udp.Test_UV_UDP) (local_addr=('127.0.0.1', 0))
----------------------------------------------------------------------
Traceback (most recent call last):
  File "uvloop/loop.pyx", line 2304, in uvloop.loop.Loop.create_datagram_endpoint (uvloop/loop.c:36698)
    udp._bind(lai.ai_addr, reuse_address)
  File "uvloop/handles/udp.pyx", line 117, in uvloop.loop.UDPTransport._bind (uvloop/loop.c:88034)
    raise exc
OSError: [Errno 49] Can't assign requested address

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/yury/dev/magic/uvloop/tests/test_udp.py", line 77, in test_create_datagram_endpoint_addrs
    transport, client = self.loop.run_until_complete(coro)
  File "uvloop/loop.pyx", line 1133, in uvloop.loop.Loop.run_until_complete (uvloop/loop.c:19943)
    return future.result()
  File "uvloop/future.pyx", line 123, in uvloop.loop.BaseFuture.result (uvloop/loop.c:94147)
    return self._result_impl()
  File "uvloop/future.pyx", line 78, in uvloop.loop.BaseFuture._result_impl (uvloop/loop.c:93686)
    raise self._exception
  File "uvloop/task.pyx", line 126, in uvloop.loop.BaseTask._fast_step (uvloop/loop.c:99430)
    result = meth(None)
  File "uvloop/loop.pyx", line 2315, in create_datagram_endpoint (uvloop/loop.c:36980)
    raise OSError('could not bind to local_addr {}'.format(
OSError: could not bind to local_addr ('127.0.0.2', 0)

Would it work if we bind local_addr to 127.0.0.1 and some unused port?

@vodik
Copy link
Copy Markdown
Contributor Author

vodik commented Aug 16, 2016

Yeah, that works. Actually, there's no reason to use a second address so long as we bind to port 0.

Still fails on master/passed with fix.

@1st1
Copy link
Copy Markdown
Member

1st1 commented Aug 18, 2016

test_create_datagram_endpoint_addrs still fails for me on MacOS, now with a TimeoutError.

Alright, could you please remove the unittest part from your PR and I'll merge it then. I'll try to figure out a way to test this properly later.

Typo causes create_datagram_endpoint to send traffic to the local_addr
instead of the remote_addr.

Closes MagicStack#45.
@vodik
Copy link
Copy Markdown
Contributor Author

vodik commented Aug 20, 2016

No problem. I don't have a mac so I can't lend a hand. Figuring out something more proper might benefit the TCP tests too.

Maybe its as simple as putting them as a different test and documenting that 127.0.0.2 needs to be added to loopback. My understanding, after googling around, is that it works on mac. Just the networking stack defaults to 127.0.0.1/32 instead of 127.0.0.1/8.

@1st1 1st1 merged commit ebf6f0b into MagicStack:master Aug 24, 2016
@1st1
Copy link
Copy Markdown
Member

1st1 commented Aug 24, 2016

Merged, thanks a lot!

@1st1
Copy link
Copy Markdown
Member

1st1 commented Aug 24, 2016

Releasing 0.5.3 right now.

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.

2 participants