Opened 10 years ago

Closed 4 years ago

Last modified 4 years ago

#2699 enhancement closed wontfix (wontfix)

UDP DatagramProtocol reconnecting

Reported by: Vazde Owned by:
Priority: low Milestone:
Component: core Keywords: UDP DatagramProtocol reconnect
Cc: Branch:
Author:

Description

I have noticed that reconnecting a UDP DatagramProtocol(maybe others, too) client is not supported. So basically what is needed is to let people to call connect() multiple times (and maybe have some way of unconnecting).

Attachments (2)

udp-reconnect-disconnect.patch (4.0 KB) - added by Martin Gergov 4 years ago.
Reconnect and disconnect a udp socket
udp-reconnect-disconnect.2.patch (7.4 KB) - added by Martin Gergov 4 years ago.
forgot the C

Download all attachments as: .zip

Change History (12)

comment:1 Changed 6 years ago by <automation>

Owner: itamarst deleted

Changed 4 years ago by Martin Gergov

Reconnect and disconnect a udp socket

Changed 4 years ago by Martin Gergov

forgot the C

comment:2 Changed 4 years ago by Martin Gergov

This patch makes udp sockets reconnect and disconnect. The disconnect sadly cannot be done in python so an optional c extension is provided.

I tested with two netcat instances - nc -ul localhost 9995 and nc -ul localhost 9999 and this client:

from twisted.internet import reactor
from twisted.internet.protocol import DatagramProtocol


class MyDgram(DatagramProtocol):

    def __init__(self, addr):
        self.addr = addr

    def startProtocol(self):
        self.transport.connect(*self.addr)

    def datagramReceived(self, data, (host, port)):
        print "Hello!{0}".format(data)

def call_it(proto):
    proto.transport.disconnect()
    print "disconnect"

def connect_dummy(proto, addr):
    proto.transport.write("Hello first server !")
    proto.transport.connect(*addr)
    proto.transport.write("Reconnect udp. Hello second !")
    reactor.callLater(20, call_it, proto)

addr = ("127.0.0.1", 9999)
addr2 = ("127.0.0.1", 9995)
proto = MyDgram(addr)
reactor.listenUDP(9998, proto)
reactor.callLater(10, connect_dummy, proto, addr2)
reactor.run()

the stdout for the socket from lsof resulted in the following sequence: UDP localhost:9998->localhost:9999 UDP localhost:9998->localhost:9995 UDP *:9998

Also, there is, again, optional unit test in twisted/test/test_udp.py

Tested only on 3.5.0-32-generic Linux !

comment:3 Changed 4 years ago by Martin Gergov

Keywords: review added

comment:4 Changed 4 years ago by habnabit

Just a note regarding your #if: twisted no longer supports versions of python older than 2.6, so it's not strictly necessary.

comment:5 Changed 4 years ago by Tom Prince

This isn't a complete review, but a couple of things:

  1. You arrange that there isnt an import error if the extension isn't availble, but the code that uses it will just fail with an unbound name error. Instead, you should raise a meaningful exception.
    • You'll need tests for this behavior. (You can do this by patching sys.modules, there are a number of examples elsewhere in the test).
    • You should skip the tests that depend on disconnect by asssining a meaningful message to .skip if the extension isn't available.
  2. The C module should have docstrings, and names exposed to python should follow the coding standard
  3. The C code needs tests for the specific functionality it is implementing.

I'm not doing a complete review, because I am unsure if this is desired behavior, or if this is the right approach for implementing it.

comment:6 Changed 4 years ago by Martin Gergov

http://bugs.python.org/issue18095 To contribute a solution to python might be a better approach

comment:7 Changed 4 years ago by Tom Prince

Keywords: review removed
Resolution: wontfix
Status: newclosed

Thanks for your contribution. However,

  1. cffi, rather than C extensions should be used for new code that needs to call in to C.
  2. it isn't clear that connected UDP is a desirable think to support,
  3. this should be handled in python itself.

comment:8 Changed 4 years ago by Jean-Paul Calderone

To be clear, please feel free to make a case that this is a feature that Twisted should support and re-open this ticket.

comment:9 Changed 4 years ago by Martin Gergov

First, sorry for not being on irc, my isp is dropping 50% of my internet traffic(slightly terrified by their technical support). Second, I guess if this feature is introduced to python it should be used in twisted when it gets to support 3.x+ so the ticket status is justified.

comment:10 Changed 4 years ago by Itamar Turner-Trauring

Connected UDP is useful insofar as you can get connection refused notification. That's about it though.

Note: See TracTickets for help on using tickets.