On Wed, Apr 6, 2011 at 12:04 PM, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
I agree with both changes, but please split that one into two separate patches.
ugh. yeah I can probably do that.
-DBUG = 1 +log_level = logging.DEBUG # logging level. set to logging.INFO to reduce output
I'm not a Python coder, but is there any reason to use lowercase here whereas we use uppercase for all other constants?
No reason really. I am just not used to uppercasing variables. When I refactor to split the above changes I will try to remember to make that variable format similar to the others.
raise SystemExit
Shouldn't we rather use "sys.exit(1);" here instead of raising a SystemExit exception? That way we'd have a proper exit status, also. Might be something to include in the debugging/error handling patch.
Possibly. sys.exit actually raises SystemExit, if I remember correctly. Setting a shell exit value is a good idea though. I will add that in.
num_comments = random.randrange(PKG_CMNTS[0], PKG_CMNTS[1]) for i in range(0, num_comments): - fortune = esc(commands.getoutput(FORTUNE_CMD).replace("'","")) + fortune = commands.getoutput(FORTUNE_CMD).replace("'","")
Why did you drop escape_string() here?
It relies upon mysql, and since the other instance of mysql usage was removed by one of my patches, I removed this as well (to remove the dep entirely). For dummy data there really isn't a danger of sql injection, and removing ' characters from the fortune_cmd result string should be enough to keep from causing the written sql to be badly formatted.