Opened 6 years ago

Closed 6 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: jesstess Branch: branches/listenmulticast-docs-4504
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

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 6 years ago by jesstess

Cc: jesstess 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 6 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 6 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 6 years ago by Jean-Paul Calderone

Owner: aalex deleted

comment:5 Changed 6 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 6 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 6 years ago by Jean-Paul Calderone

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 6 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/listenmulticast-docs-4504

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

comment:9 Changed 6 years ago by Jean-Paul Calderone

(In [29630]) Apply aalex's patch

refs #4504

comment:10 Changed 6 years ago by Jean-Paul Calderone

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 6 years ago by Itamar Turner-Trauring

Keywords: review removed
Owner: set to Jean-Paul Calderone

"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 6 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(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 6 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.