Ticket #4504 enhancement closed fixed

Opened 3 years ago

Last modified 3 years ago

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
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

1

Changed 3 years ago by jesstess

  • keywords documentation, easy added; docstring removed
  • cc jessica.mckellar@… added
  • owner changed from glyph to aalex

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

2

Changed 3 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

3

Changed 3 years ago by aalex

  • keywords easy, review added; easy removed

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

4

Changed 3 years ago by exarkun

  • owner aalex deleted

5

Changed 3 years ago by jesstess

  • keywords easy added; easy, 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. :)

6

Changed 3 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!

7

Changed 3 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. :)

8

Changed 3 years ago by exarkun

  • branch set to branches/listenmulticast-docs-4504
  • branch_author set to exarkun

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

9

Changed 3 years ago by exarkun

(In [29630]) Apply aalex's patch

refs #4504

10

Changed 3 years ago by exarkun

  • keywords documentation review added; documentation, removed
  • 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

11

Changed 3 years ago by itamar

  • owner set to exarkun
  • keywords review removed

"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.

12

Changed 3 years ago by exarkun

  • status changed from new to closed
  • resolution set to fixed

(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.

13

Changed 2 years ago by <automation>

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