Alon Bar-Lev has posted comments on this change. Change subject: fklsnr: Introduce standalone fence_kdump listener ......................................................................
Patch Set 6: (3 comments) 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? I do not want to go to sleep less than second as it just consumes cpu without actual need, so the max(time_left, 1) if the interval is float as it supported in python for some reason(?) apart from that the code is the same, no? replace one time used block in another... and add lots of temp variables. I am fine to have this as function (without the tmep variables), as long as this function gets the interval and last wakeup and not directly use the members as it can potentially be reused... but as long as it does not reused I do not see a benefit. 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()? my code is usually as much as python3 compatible as I can. 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. it is used only here at session house keeping... not sure why we need a separate function that people may be confused to call. I do not know that () can do inner loops.... can you please describe the syntax? 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
