[Nym3-devel] Re: [Nym3-commit] r454 - in trunk/nymbaron: . Server

[jr at komite.net]

Quoting zax at conuropsis.org:

> Author: zax
> Date: 2006-03-06 16:48:22 +0100 (Mon, 06 Mar 2006)
> New Revision: 454
>
> Modified:
>   trunk/nymbaron/Mail.py
>   trunk/nymbaron/Server/Main.py
> Log:
> Minor change to relay function that will make it easier to test and 
> debug other relay methods.
>
>
> Modified: trunk/nymbaron/Mail.py
> ===================================================================
> --- trunk/nymbaron/Mail.py	2006-02-27 18:06:16 UTC (rev 453)
> +++ trunk/nymbaron/Mail.py	2006-03-06 15:48:22 UTC (rev 454)
> @@ -170,13 +170,8 @@
>     body = filter_header(body)
>     b = 'From: ' + nym + '@' + sname + "\n\n" + body
>     fname = tmpFileMsg(b) #TODO we can avoid the tempfile with some 
> fd manipultation : exec mixminion, write b in its stdin
> -    mixcmd = "mixminion send -t 0x" + b2s(rt[0]) + b2s(rt[1]) + ":" 
> + ri + " -i " + fname
> -    print mixcmd
> -    ec = os.system(mixcmd)
> -    # TODO : debugging cruft
> -    #os.unlink(fname)
> -    print "Raw relayed message left in " + fname
> -    return ec
> +    relaycmd = "mixminion send -t 0x" + b2s(rt[0]) + b2s(rt[1]) + 
> ":" + ri + " -i " + fname
> +    return relaycmd
>
> def genMid(length = midLen):
>     """Randomly generates a mid of given length different from oldestMid"""
>
> Modified: trunk/nymbaron/Server/Main.py
> ===================================================================
> --- trunk/nymbaron/Server/Main.py	2006-02-27 18:06:16 UTC (rev 453)
> +++ trunk/nymbaron/Server/Main.py	2006-03-06 15:48:22 UTC (rev 454)
> @@ -240,9 +240,13 @@
> 		    elif (com.ct() == Message.CToSCODE['Newpk']):
> 			nymUser.setKeys(com.kid,com.kenc)
> 		    elif (com.ct() == Message.CToSCODE['Relay']):
> -			ec = Mail.relay(h.nym, com.rt, com.ri,
> +			relaycmd = Mail.relay(h.nym, com.rt, com.ri,
> 					config.serverName, com.body)
> -			if (ec != 0):
> +			logger.debug('Attempting to relay nymholder message using command 
> %s' % relaycmd)
> +			ec = os.system(relaycmd)
> +			if (ec == 0):
> +			    logger.debug('Message relay successfully processed')
> +			else:
> 			    logger.error("mixminion exited abnormally with error code %d" % ec)
> 			    sys.exit(2)
>


I don't think this is a minor change. In order to log the command being 
executed  the command isn't executed in the function it was meant to be 
anymore. As a consequence the clean up isn't done anymore, and nothing 
advertises it is missing. (the temporary file should be unlinked when 
we are not in debug mode, that was what the debugging cruft was 
pointing at, it dates back from when we hadn't logging and debug mode 
:))

I would find it more appropriate if the logging was done inside 
Mail.relay, but I haven't looked yet at how the logging function works. 
Can a function in Mail.py access it easily? It would also be nice if 
relay could access to the config object to know wheter it should unlink 
the temporary file or not. Maybe relay shouldn't be in Mail if these 
things are difficult to do...

JR