[Twisted-Python] Re: nntp protocol supports moderated newsgroups?

[Jp Calderone]

Barry A. Warsaw wrote:

>>>>>>"JC" == Jp Calderone <exarkun at twistedmatrix.com> writes:
>>>>>>            
>>>>>>
>
>    JC>  Hmm, twisted/news/database.py should have all the stuff it
>    JC> needs already - maybe makeGroupSQL() will need to be modified
>    JC> a bit so that twisted/tap/news.py can tell it which groups
>    JC> should be moderated and which shouldn't.  twisted/tap/news.py
>    JC> will probably have most of the action; it'll need code to let
>    JC> the user specify moderated/unmoderated to mktap somehow, maybe
>    JC> with another --groups parameter, or with some indicator in the
>    JC> groups file.
>
>    JC> Maybe it's time twisted.news got a *real* configuration
>    JC> interface :) I'll ponder that a little (maybe something fun
>    JC> with woven), and if you(/anyone else) have any ideas on this,
>    JC> I'd love to hear them.
>
>I don't really, since I'm fairly green in my Twisted use.  I will note
>that the news stuff seems a little more difficult to use than when I
>last looked at it (Twisted 0.19 I think).  I'm using Twisted from cvs
>now, so it's possible this stuff is just in flux.
>  
>
  I wish I could say that is the case -- I've just been motivated to 
work on other
projects lately.  I agree that it has gotten more complex, and I don't 
like it.  I've
recently been pointed at twisted.persisted.dirdbm, and my current 
thinking is
that PickleStorage and NewsStorageAugmentation should go away in favor of
a DirDBMStorage.  PickleStorage was never meant as anything more than a
toy, and the SQL stuff doesn't seem to have yielded any performance 
benefits,
probably due to the complexity of some of the common queries it is 
performing.

>Specifically:
>
>- What is the --servers switch for?  I couldn't actually find where
>  that was used in tap/news.py but it seemed required, so I fed it
>  nonsense. 
>
The file given to --servers should contain a list of news servers to which
to connect and share messages with.  This hasn't yet been tested 
thoroughly --
I have surprising difficulty finding a news server that implemented 
enough of
the protocol to let this work.  I guess the "user facing" servers are kept
intentionally non-functional for some reason.

>
>- I understand why "mktap news" requires an extra argument now, but it
>  would be nice if either the pickle or sql command were defaulted.  I
>  don't think I needed to specify "mktap news pickle" last time.  Hmm,
>  maybe if --file is given you can default to pickle?
>
Yea, this is new too, since pickle was the only legal command in the 
past.  If I
go with the dirdbm stuff, I'll probably make it the default and let 
pickle or sql
be specified if someone wants something different (Or pickle and sql might
quietly disappear - the prospect of maintaining 3 different backends - 2 of
which are likely useless to nearly everybody - is a little unpleasant).

>
>- I noticed some bugs in the code, a few of which I fixed, but some I
>  didn't.  I ran the server w/ twistd -n and watched the output.  I
>  got mysterious log messages such as:
>
>[...]
>19/10/2002 11:54 [*news*] Starting factory <twisted.news.news.UsenetClientFactory instance at 0x82a52d4>
>19/10/2002 11:54 [*news*] Connection failed: [Failure instance: Traceback! exceptions.IOError, address not found
>19/10/2002 11:54 [*news*] ]
>19/10/2002 11:54 [*news*] Connection failed: [Failure instance: Traceback! exceptions.IOError, address not found
>19/10/2002 11:54 [*news*] ]
>19/10/2002 11:54 [*news*] Stopping factory <twisted.news.news.UsenetClientFactory instance at 0x82a52d4>
>19/10/2002 11:55 [*news*] Starting factory <twisted.news.news.UsenetClientFactory instance at 0x82a52d4>
>[...]
>
>  While these repeated as long as the server was running, they didn't
>  seem to affect its operation.
>  
>
I'm guessing these are due to whatever you put in your server list file.

>- In news/news.py, buildProtocol() is trying to pass self.remoteHosts
>  to the self.protocol() call, but self.protocol() -- which is really
>  the nntp.NNTPServer class, doesn't take two arguments.
>
Thanks for the fix.

>In any event, I hacked in moderated group support by adding a
>-m/--moderators flag, which takes a file containing a list of
>newsgroup/moderator address pairs.  When a message is POSTed to a
>group, the list of target newsgroups is checked against the
>moderators.  If there is any match, and the article has no Approved
>header, the article is not posted, but instead emailed to the
>moderators address.  The value of the Approved header is completely
>ignored -- it just has to be there.  I believe this is pretty close to
>how Usenet s/w handles moderators.
>  
>
Hmm.  How does a moderator then approve the message?  The interface
I had envisioned was something closer to mailman's scheme, though this
is certainly easier on the server side :)

>A couple of kludges that should be cleaned up:
>
>- What to do if the list of target newsgroups has more than one
>  moderated newsgroup in it?  Off hand, I'm not sure what Usenet does
>  in this case.
>
I believe the correct behavior would be to post the message to all 
un-moderated
groups and hold the message for approval from the moderators of each 
moderated
group, posting it to each held group when the appropriate moderator 
approves it.
One issue I see with this, though, is if someone is a moderator for 
multiple groups
that a single message is sent to.  I'm not sure if/how you could/would 
approve the
message for one group but reject it for another.  Maybe once I 
understand how the
email approval system works this will become clear.

>
>- We really need a few more config options, including the smtp host
>  and port to use to send out the moderator's email, and the envelope
>  sender address to use in that email.  Both are hard coded in my
>  hack.
>
Indeed.

>
>- Any un-Twisted coding offenses. :)
>  
>
The only one I see is the use of smtplib.  Its sendmail method will very 
likely
block, possibly for a long time, hanging the entire server.  Aside from 
just being
nasty, this opens up the possibility for DoS on servers, simply by 
repeatedly mailing
a moderated group.  I'm going out of town for a week this coming 
tuesday, and I
doubt I'll have time to figure out how twisted.protocols.smtp.SMTPClient 
works
before then, so I'm going to hold off on committing this.  If you want 
to figure that
out and send another patch, that'd be great, otherwise I'll just take 
care of it when
I get back.

>So, in the grand tradition of scratching my own itch, but just enough
>so I can sleep, here's the patch.
>
>Oh, BTW, for someone who was completely green to the code base,
>Twisted is very nicely written.  It wasn't difficult at all to drill
>down to where I needed to make the changes.  JP's hints helped a lot
>too.  Yay!
>  
>
  Glad to hear it :)  Thanks for the patch.

  Jp