[Twisted-Python] Re: Where is best place to put my custom code?

[Andrew Bennetts]

On Fri, Jan 02, 2004 at 06:13:39PM -0800, JD wrote:
> On Jan 1, 2004, at 6:52 PM, Andrew Bennetts wrote:
> >On Thu, Jan 01, 2004 at 11:25:37AM -0800, JD wrote:
> >>
> >>do I use:    testIRC.sendLine('signedOn')  to send the command to the
> >>IRC server
> >>to signon?
> >
> >No.  IRCClient is a LineReceiver subclass, because that's how the 
> >low-level
> >protocol works.
> 
> I think I understand.   what is needed in the docs,   is a better
> description of just what the roles of these objects are supposed to do,
> and where we,  as users of Twisted,  would have a much better chance to
> use Twisted in the way it is intended.    So the usage of 'sendLine' in a
> 'lineReceiver' would be inappropriate?  but if that's true,  then how does
> one send a command to the IRC server?

No, using sendLine in a LineReceiver is fine -- that's why LineReceiver
defines sendLine.

What isn't so good is using sendLine in an IRCClient subclass.  While
IRCClient subclasses LineReceiver, in general you should be using the
abstractions IRCClient provides to send commands to and receive responses
from the server (if you don't want to use them, then why not just use a
plain LineReceiver?).

The only reason you should have for calling sendLine in an IRCClient
subclass is to extend its functionality to support more of the protocol than
it already does.  IRCClient already has support for signing on (the
'register' method and the 'signedOn' handler), so you shouldn't use sendLine
for that.  IRCClient currently lacks some IRC features, such as WHO, so to
do those you do need sendLine.

Also, good style suggests that you probably shouldn't mix your application
code with your IRCClient extensions, i.e. subclass IRCClient for WHO support
(and other other unimplemented commands you need), and then subclass that
for your application.

> By the way,   I've chosen this as a simple test application,  not unlike
> what I want to really do,   so I have chosen a relatively simple
> application,  where I just extract all the users of a specified IRC
> server,  and return the data.
> 
> I have not specified the form of the data,  that's not important,  but
> what's important to me,  is the understand how to use Twisted in the way
> it was intended.
> 
> I have this:
> 
> class IRC_test(irc.IRCClient):
> 
>     def whois_on(self):
>     	"""
>     	This method issues a 'WHO' command to see who is on the server.
>     	"""
>     	self.sendLine("WHO")
> 
>     def signedOn(self):
>         """Called after sucessfully signing on to the server.   So we
>         immediately issue the 'whois_on' message to let the server go and
>         do it's stuff.
>         """
>         self.whois_on(self)
> 
>     def whois_result(self, prefix,  info):
> 		"""
> 		This is the code the IRC is somehow going to call.   Then,
> 		after it is called,  it stores the info in the factory
> 		and returns by calling 'reactor.stop()' to exit the 
> 		reactor.run()
> 		"""
> 		self.result = extract_whois_data(info)
> 		reactor.stop()

(This identation is really painful.)

>     def irc_RPL_WHOISSERVER(self, prefix, params):
>     	"""
>     	This was not included in the parent class IRCClient,
>     	So I add it here.  'lineReceived' gets the
>     	command = numeric_to_symbolic[command],  which is now
>     	the string 'RPL_WHOISSERVER'.   To get THIS method to call,
>     	the 'handleCommand' method first generates a key to the
>     	class's 'attribute' dictionary,  by pre-pending 'irc_'
>     	in front of it to get this method,  which is then called.
>     	'params' would then have the result data.   We need to
>     	hack around to see what form it's in.
>     	"""
>         self.whois_result(params[1])

(This identation won't even work!)

But creating a method called 'irc_RPL_WHOISSERVER' will do what you want
here.  Handler methods with a common prefix that are dispatched with a
getattr call are a common, and very nice, idiom in Twisted.

> def test_irc(host, port):
> 	"""
> 	This function scans a particular IRC server,  and returns a
> 	dictionary of all the nicks and IP addresses.  Nick is the key,  the
> 	IP or reverse dns is the value.
> 	"""
> 	my_nick = "jd6969"
> 	data_dict = {}		# place where I want my IRC data
> 	myTestFactory = IRC_test(data_dict,  my_nick)
> 	reactor.connectTCP(host, port, myTestFactory)
> 	reactor.run()		# I just wait for the server to respond
> 	return data_dict	# when it does, I just want to return with the data

(Ugh, and now you're using tabs...)

IRC_test is a protocol, but you're passing it as a factory to connectTCP.
See http://twistedmatrix.com/documents/howto/clients.

> I know this is a pretty stupid and useless application,  but this is very
> close to what I want it to do,  without revealing anything.    I haven't
> tested this yet,  because I just want to be sure I'm doing it right,
> rather then to spend weeks down a particular path to nowhere.

This code is short.  Why didn't you run it yourself to get some idea of how
close it is to working?  Working code is usually a good sign that you're
close to the right path, if not already on it...

> >However, you're not using the low-level protocol, you're
> >using an abstraction on top of it -- IRCClient.  Don't break the 
> >abstraction
> >(by calling LineReceiver's sendLine method) unless you have a good 
> >reason to
> >and you know the protocol spec well enough to know what you're doing 
> >(which
> >would be RFC 2812 in this case, I think).
> 
> I believe I do,  because I still have to tell the IRC server what I want
> it to do.  And the only way I can see,  is with the sendLine command.

Only because you need to extend it to cover parts of the protocol that
aren't implemented.  You started off talking about 'signedOn', which is
already covered by IRCClient, and only later mentioned trying to do a WHO
command, which isn't.  Confusing issues like this makes it hard to figure
out what you're trying to do.

> >As far as I'm aware, sending a line saying 'signedOn' to the server 
> >doesn't
> >make any sense in the IRC protocol, so this definitely wouldn't achieve
> >anything useful, except causing an error from the server that the 
> >IRCClient
> >instance probably won't expect (because you broke the abstraction and 
> >snuck
> >behind its back to do it).
> 
> Ok,   so it is clear I'm doing this wrong.  If that's the case,  then what
> is the RIGHT way of doing it?

Calling register, of course.  Where in the RFC does it say 'signedOn' is a
recognised command?

> >>or do I:   testIRC.register('mynick', 'myhost', 'irc.debian.com')
> >
> >That looks much saner.  And noticeably more full of information than
> >testIRC.sendLine('signedOn')...
> >
> >>then,   when I want to do the "who" command,  would I do:
> >>testIRC.sendLine('who')
> >
> >Well, I believe the WHO command requires an argument, so it would be 
> >more
> >like testIRC.sendLine('WHO ' + name)... but see my next answer.
> 
> hmmm - in checking the RFC it mentions I can either "WHO <channel>" or
> "WHO" without specifying a channel,  in which it gets the names of ALL the
> users of the IRC server not hidden.

Fair enough.  Believe it or not, but I actually know very very little about
IRC, I mainly figured it out for that email to you by skimming irc.py and
one or two brief google searches to confirm e.g. that 'signedOn' doesn't
appear in the RFC.

> >The right way to do this is implemented in sandbox/exarkun/irc2.py -- 
> >see
> >the AdvancedClient.who method.  I'd follow JP's earlier suggestion and 
> >use
> >it.
> 
> I was NOT aware that anything in "sandbox" had anything to do with IRC.
> Expecially when words like this are chosen.   One can interpret 'sandbox'
> in many zillion ways,  so how would I even know this.

Because of a mail from earlier in the thread from JP:
    http://twistedmatrix.com/pipermail/twisted-python/2004-January/006893.html

Which part of

    "WHO is one of the commands I did manage to improve, but I never moved
    any of the work out of my sandbox.  You can find it in
    Twisted/sandbox/exarkun/irc2.py."

was unclear?

> I took a look at this,  and WOW - this might be just what I need.   I'm
> going to have to spend a lot of time studying this.    I had no clue this
> even existed, primarily because the name 'sandbox' really had nothing to
> do with IRC, and I just couldn't make the connection.

Hence the email.

> >The answer is of course none of those.  You should've already noticed 
> >that
> >IRCClient doesn't implement a who() method, so it's hardly surprising 
> >it
> >hasn't implemented a command to deal with its response.
> 
> yes - as it also doesn't implement a LOT of things,  and yet they are
> defined as overrideable functions.

I don't understand what you're saying here.  You mean things like
irc_RPL_WHOISSERVER?  That's just the way the prefixed getattr idiom works;
all you need to do support a new command define a handler method for it, and
that's it.  No explicit registration or anything like that needed.  Python's
a dynamic language, and Twisted uses that to its advantage.

> >I thought you wanted to use the WHO command not WHOIS?  Please try to 
> >be
> >precise.
> 
> I thought they were almost the same....  at least that's what I read in
> the RFC's.  What is the difference?

No idea.  Again, I don't know IRC very well :)

I notice that irc2.py implements both WHO and WHOIS seperately, though, so I
figure there are some differences, which is why you saying WHO and then
later WHOIS confused me.

> >>Could I add this to my IRCClient's subclass?
> >>
> >>IE:
> >>
> >>    def irc_RPL_WHOISSERVER(self, prefix, params):
> >>        self.whois(params[1])
> >
> >If you look at irc2.py, you'll see it does override this method.  Note 
> >that
> >'whois' is a bad name for the response handler -- the existing 
> >convention in
> >IRCClient is that 'join', 'nick', etc, are the methods that *issue* the
> >command, not that handle the response.
> 
> I haven't seen any mention of naming conventions,  but if there is one,
> I certainly would want to know where it is.

It's not explicitly written anywhere that I know of.  It was just obvious to
me from skimming the code of irc.py and irc2.py that that's what the
convention was, because that's what the existing code *does*.  I generally
try to follow the conventions of code I'm using to avoid getting horribly
confused.

> >I think the biggest critical step you're missing is that you're busily
> >trying to reinvent a wheel that's already been made in
> >sandbox/exarkun/irc2.py, though.
> 
> that is obviously true....  I explained above why I just didn't even know
> of it's existance until it was pointed out to me.

Yes, but it was pointed out to you before this...

-Andrew.