Opened 4 years ago

Closed 4 years ago

#4504 enhancement closed fixed (fixed)

Document reactor.listenMulticast's listenMultiple keyword argument

Reported by: aalex Owned by:
Priority: normal Milestone:
Component: core Keywords: documentation easy
Cc: jessica.mckellar@… Branch: branches/listenmulticast-docs-4504
(diff, github, buildbot, log)
Author: exarkun Launchpad Bug:

Description

@param listenMultiple: If set to True, allows multiple servers to listen to the same address and port of a multicast group.
@type listenMultiple: bool

Some commonly used addresses are the All Hosts multicast group, such ass 224.0.0.1, that contains all systems on the same network segment.

Change History (13)

comment:1 Changed 4 years ago by jesstess

  • Cc jessica.mckellar@… added
  • Keywords documentation easy added; docstring removed
  • Owner changed from glyph to aalex

aalex is going to give a go at a patch today! ==> Reassigning.

comment:2 Changed 4 years ago by aalex

Here is my patch:

Index: udp.py
===================================================================
--- udp.py	(revision 29319)
+++ udp.py	(working copy)
@@ -309,6 +309,12 @@
     implements(interfaces.IMulticastTransport)
 
     def __init__(self, port, proto, interface='', maxPacketSize=8192, reactor=None, listenMultiple=False):
+        """
+        @type listenMultiple: bool
+        @param listenMultiple: If set to True, allows multiple servers to listen to the same address and port of a multicast group.
+
+        Some commonly used multicast addresses are the All Hosts multicast group, such ass 224.0.0.1, that contains all systems on the same network segment.
+        """
         Port.__init__(self, port, proto, interface, maxPacketSize, reactor)
         self.listenMultiple = listenMultiple

comment:3 Changed 4 years ago by aalex

  • Keywords review added

I updated my comment a bit:

Index: twisted/internet/udp.py
===================================================================
--- twisted/internet/udp.py	(revision 29319)
+++ twisted/internet/udp.py	(working copy)
@@ -309,6 +309,14 @@
     implements(interfaces.IMulticastTransport)
 
     def __init__(self, port, proto, interface='', maxPacketSize=8192, reactor=None, listenMultiple=False):
+        """
+        @type listenMultiple: C{bool}
+        @param listenMultiple: If set to True, allows multiple servers to listen to the same address and port of a multicast group.
+
+        Some commonly used multicast addresses are the All Hosts multicast group, such ass 224.0.0.1, that contains all systems on the same network segment.
+        
+        To join a multicast group, it might be useful to create a subclass of t.p.DatagramProtocol in order to override its startProtocol method. In it, one can call self.transport.joinGroup(224.0.0.1) to join the desired multicast group, which is 224.0.0.1 in this example.
+        """
         Port.__init__(self, port, proto, interface, maxPacketSize, reactor)
         self.listenMultiple = listenMultiple

comment:4 Changed 4 years ago by exarkun

  • Owner aalex deleted

comment:5 Changed 4 years ago by jesstess

  • Keywords review removed
  • Owner set to aalex

Thanks for working on this, aalex! Some feedback:

  • Please line wrap at 79 columns. More on Twisted's coding conventions can be found here
  • I would leave out the example and let people reference the RFC if they'd like to learn more about multicast.
  • Since this docstring is already getting added, can the rest of the parameters get markup too? I'd say "steal from Port", but Port doesn't document its init parameters either (if you're up for both, even better!).

Please attach subsequent diffs instead of pasting into a comment. :)

comment:6 Changed 4 years ago by jesstess

I'm also happy to do the parts of the feedback you don't have time for. Thanks again for the patch!

comment:7 Changed 4 years ago by exarkun

Also, some of this documentation looks like it would be better off in doc/core/howto/udp.xhtml. epytext markup for parameters usually follows the descriptive part of a docstring, rather than the other way around. And don't forget a news fragment. :)

comment:8 Changed 4 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/listenmulticast-docs-4504

(In [29629]) Branching to 'listenmulticast-docs-4504'

comment:9 Changed 4 years ago by exarkun

(In [29630]) Apply aalex's patch

refs #4504

comment:10 Changed 4 years ago by exarkun

  • Keywords review added
  • Owner aalex deleted

I changed things around a bit. There is api doc coverage for listenMultiple already. I'm not sure why aalex was looking in twisted/internet/udp.py instead of at the interface. I put a @see in to try to help out the matter.

Build results

comment:11 Changed 4 years ago by itamar

  • Keywords review removed
  • Owner set to exarkun

"all hosts group" is a bit mystifying, how about "the special address for all hosts on the local network segment (see http://www.faqs.org/rfcs/rfc1112.html)" if that is accurate? We probably should have a reference to the RFC in the howto regardless.

Other than that (and need for news file) looks good.

comment:12 Changed 4 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(In [29800]) Merge listenmulticast-docs-4504

Author: aalex, exarkun
Reviewer: itamar
Fixes: #4504

Improve the discoverability of information about multicast, in particular
the listenMultiple argument to IReactorMulticast.listenMulticast. Also
drop a reference to the RFC into the UDP howto.

comment:13 Changed 4 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.