Saggi Mizrahi has posted comments on this change. Change subject: fklsnr: Introduce standalone fence_kdump listener ......................................................................
Patch Set 6: (3 comments) I could keep on nitpicking but I don't really have time for that. The only thing I noticed is that this version doesn't really handle the fact that the DB might be slow or have occasional disconnects. Seems a bit odd as the last version had a lot of facilities for such errors. http://gerrit.ovirt.org/#/c/27201/6/packaging/services/ovirt-fence-kdump-listener/listener.py File packaging/services/ovirt-fence-kdump-listener/listener.py: Line 82: Line 83: def _recvfrom(self): Line 84: ret = (None, None) Line 85: Line 86: try: Why not do it like this? def _get_remaining_interval(): last = self._lastWakeup interval = self._wakeupInterval total_seconds = self._total_seconds(self._now() - last) return interval - total_seconds time_left = self._get_remaining_interval() if time_left <= 0: raise socket.timeout() self._socket.settimeout(time_left) .... Line 87: if self._interval_finished( Line 88: interval=self._wakeupInterval, Line 89: last=self._lastWakeup, Line 90: ): Line 203: ) Line 204: self.logger.debug('Exception', exc_info=True) Line 205: Line 206: def _house_keeping_sessions(self): Line 207: for session in self._sessions.values(): The is only one thread, why not use itervalues()? Line 208: received = self._now() Line 209: Line 210: if self._interval_finished( Line 211: interval=self._sessionExpirationTime, Line 245: ) Line 246: ) Line 247: self.logger.debug('Exception', exc_info=True) Line 248: Line 249: for address in [ could be () instead of [] since you already copied the session list. Also, might be better to move to it's own method self._clear_closed_sessions() Line 250: entry['address'][0] Line 251: for entry in self._sessions.values() Line 252: if entry['status'] == self.SESSION_STATE_CLOSED Line 253: ]: -- 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: 6 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
