Alon Bar-Lev has posted comments on this change. Change subject: fklsnr: Introduce standalone fence_kdump listener ......................................................................
Patch Set 13: (6 comments) last minor comments! almost there. http://gerrit.ovirt.org/#/c/27201/13/packaging/services/ovirt-fence-kdump-listener/db.py File packaging/services/ovirt-fence-kdump-listener/db.py: Line 199: status, # v_status Line 200: json.dumps(address), # v_address Line 201: ), Line 202: ) Line 203: return res[0]['upsertkdumpstatusforip'] == 1 if res else False how can res be nothing? I expect it to be either error (exception) or # of lines. we do call stored procedure... Line 204: Line 205: def update_heartbeat(self): Line 206: return self._db_mgr.call_procedure( Line 207: name='UpsertExternalVariable', http://gerrit.ovirt.org/#/c/27201/13/packaging/services/ovirt-fence-kdump-listener/listener.py File packaging/services/ovirt-fence-kdump-listener/listener.py: Line 189: ) Line 190: ) Line 191: # remove finished sessions (engine will remove them from db) Line 192: if session['status'] == self.SESSION_STATE_CLOSED: Line 193: del self._sessions[session['address']] I am unsure you can delete the session while iterating. you can copy the values: for session in self._sessions.values()[:]: or create a list of keys to delete: for address in ( session['address'] for session in self._sessions if session['status'] == self.SESSION_STATE_CLOSED ): del self._sessions[address] Line 194: Line 195: def _heartbeat(self): Line 196: if self._interval_finished( Line 197: interval=self._heartbeatInterval, Line 257: if address not in self._sessions: Line 258: session = self._create_session( Line 259: status=self.SESSION_STATE_DUMPING, Line 260: address=address, Line 261: updated=datetime.datetime.utcnow(), I think you can put this now() as default within session every time you create. Line 262: dirty=False, Line 263: ) Line 264: self._sessions[session['address']] = session Line 265: Line 260: address=address, Line 261: updated=datetime.datetime.utcnow(), Line 262: dirty=False, Line 263: ) Line 264: self._sessions[session['address']] = session this should be within the if scope, bad python not having scopes! Line 265: Line 266: self._afterFirstDbSync = True Line 267: Line 268: def _db_sync(self): Line 279: self._db_connection_valid = True Line 280: try: Line 281: self._heartbeat() Line 282: self._save_sessions() Line 283: self._load_sessions() any reason why load is after save? not that it is that important... but in some logical sense... :) Line 284: except db.DbException as e: Line 285: self.logger.debug( Line 286: ( Line 287: "Error during synchronization with database, " Line 298: "Database connection is not available, " Line 299: "synchronization will be postponed." Line 300: ) Line 301: ) Line 302: self._lastDbConnReopen = datetime.datetime.utcnow() lastDbConenctionAttempt ? Line 303: Line 304: def run(self): Line 305: while True: Line 306: (data, address) = self._recvfrom() -- To view, visit http://gerrit.ovirt.org/27201 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieec3bad47bbba860a52a9ff4e2eb7f61277f4e36 Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
