Code review comment for lp:~mirsad-vojnikovic-deactivatedaccount/lava-dispatcher/scheduler-daemon

Revision history for this message
Spring Zhang (qzhang) wrote :

110 + message = "pidfile %s does not exist. Daemon not running?\n"
111 + sys.stderr.write(message % self.pidfile)
It's better to place message in one line, or it seems the message is split.Like:
message = "pidfile %s does not exist. Daemon not running?\n" %self.pidfile

It seems your alignment is by 8 spaces/<tab>, I remember it is 4 spaces. Can Paul confirm?

155 + self.connection = sqlite3.connect('./scheduler/database.db')
Can it be a constant variable for './scheduler/database.db'? Not hardcode in function

71 + file(self.pidfile,'w+').write("%s\n" % pid)
The file object needs to close?

150 + def __init__(self):
151 + self.connection = None
152 + self.cursor = None
153 +
154 + def connect_db(self):
155 + self.connection = sqlite3.connect('./scheduler/database.db')
156 + self.cursor = self.connection.cursor()
Can the two methods merge to __init__()?

207 + db_helper = DbHelper()
And can the database name call from caller by a parameter, not specify in DbHelper class?

188 + #params = cmd.get('parameters', {})
189 + #step = lava_commands[cmd['command']](target)
190 + #step.run(**params)
191 +
192 +
Please delete unused statement.

You can use the dispatcher.py from lp:lava, just merge from lp:lava in your branch.

« Back to merge proposal