Martin Peřina has posted comments on this change. Change subject: fklsnr: Introduce standalone fence_kdump listener ......................................................................
Patch Set 13: (6 comments) 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 Well, you're probably right, I think we can remove this failsafe ... 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're right, I forgot that Python 3 will return iterator instead of new list when calling values() ... 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 cre Done 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! Done 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 Well, memory session contains more recent data, so when I save them to db first, I will probably read less unfinished sessions during load ... But the difference is not big :-) 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 ? Done 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 <mper...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches