Ticket #5574 enhancement new

Opened 14 months ago

Last modified 3 weeks ago

Add support for SOCK_DGRAM to IReactorSocket

Reported by: exarkun Owned by:
Priority: normal Milestone:
Component: core Keywords: review
Cc: Branch: branches/udp-port-from-fd-5574
Author: rwall Launchpad Bug:

Description

#5248 added support for some taking some existing SOCK_STREAM listening sockets and adding them to the reactor.

A similar feature would be support for adding an existing SOCK_DGRAM socket to the reactor.

This would probably be exposed as a new IReactorSocket method.

Change History

1

Changed 3 weeks ago by rwall

  • owner set to rwall
  • status changed from new to assigned

2

Changed 3 weeks ago by rwall

  • branch set to branches/udp-port-from-fd-5574
  • branch_author set to rwall

(In [38323]) Branching to 'udp-port-from-fd-5574'

3

Changed 3 weeks ago by rwall

I've taken most of the changes made that were made for adoptStreamPort in #5248 and applied them to the udp.

TODO:

  • Fix twistedchecker problems
  • Add maxPacketSize to interface
  • Add tests for maxPacketSize
  • Write a script to do some functional testing with systemd
  • check why udp.Port.startListening doesn't have the same addressFamily check and _resolveIPv6 call as tcp.Port

Build Results look promising:

4

Changed 3 weeks ago by rwall

  • keywords review added
  • status changed from assigned to new
  • owner rwall deleted

Ready for review in log:branches/udp-port-from-fd-5574

  1. This isn't very original work, I've just copied and adapted the changes in #5248 for UDP sockets.
  2. Added an IReactorSocket.adoptDatagramPort method - I considered calling it adoptDGRAMPort but thought it looked ugly.
  3. Refactored the test_udp.py module so that tests can be reused for both newly created UDP sockets and adopted sockets.
  4. Added a verifyClass test for IReactorSocket because I'd forgotten to add the maxPacketSize argument to the IReactorSocket. (although I expected that to be built into the ReactorBuilder since you have to specify "requiredInterfaces").
  5. There is a verifyClass test for MemoryReactor which is why I added adoptDatagramPort to that - even though I don't use it in this branch.
  6. Improved the docstrings in listenUDP and udp.Port.init.
  7. tcp.Port.startListening has special handling of IPv6 address "scope identifiers" but udp.Port does not. Is it relevant for UDP? Probably a separate ticket anyway.
                    if self.addressFamily == socket.AF_INET6:
                        addr = _resolveIPv6(self.interface, self.port)
                    else:
                        addr = (self.interface, self.port)
    
  8. There was already a lot of duplication in test_socket module, and I've made it worse. I considered factoring out the common tests and implementing mixins containing getListeningPort for the DGRAM and STREAM sockets - let me know if that makes sense in this branch.

Build Results:

  1.  http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/udp-port-from-fd-5574
  2. documentation builder seems to have highlighted a bunch of existing errors instead of giving a "new errors" list. I think they can be ignored.
  3. twistedchecker should ignore the missing @param etc in TestMixin classes - I think they can be ignored.
  4. I deliberately didn't document PosixBaseReactor.adoptDatagramPort so that the full API docs can be inherited from IReactorSocket.

-RichardW.

Note: See TracTickets for help on using tickets.