Saggi Mizrahi has posted comments on this change. Change subject: fklsnr: Introduce standalone fence_kdump listener ......................................................................
Patch Set 4: (2 comments) http://gerrit.ovirt.org/#/c/27201/4/packaging/services/ovirt-fence-kdump-listener/db.py File packaging/services/ovirt-fence-kdump-listener/db.py: Line 175: def load_config_values(self): Line 176: config_values = {} Line 177: self.open_connection() Line 178: try: Line 179: for name in ( > but isn't that why we have traceback information within the log? As I said, you don't always have debug level logs, people not always log as in my example since it's expected behavior. Also, logs don't solve having solution next to problem. And expressing intent. Line 180: 'NextKdumpTimeout', Line 181: 'KdumpFinishedTimeout', Line 182: 'FenceKdumpListenerHost', Line 183: 'FenceKdumpListenerPort', http://gerrit.ovirt.org/#/c/27201/4/packaging/services/ovirt-fence-kdump-listener/logbase.py File packaging/services/ovirt-fence-kdump-listener/logbase.py: Line 16: Line 17: from ovirt_engine import base Line 18: Line 19: Line 20: class Logable(base.Base): > this is not required, we use Base not this one, see[1], with no magic. base.Base should have been a decorator. Not a base class. You now have countless classes inheriting from a class assuming it does nothing. Any addition\change would be hard to do since it'll be impossible to assume what all the countless subclasses are doing. You effectively said everything is a specialization of "class with log" which makes no sense. logging is a horizontal not a vertical trait and this is why you use horizontal language features like decorators\meta-classes and not vertical features like inheritance. But I'm not reviewing the base.Base. Line 21: def __call__(self, target): Line 22: _logger = logging.getLogger(self._LOG_PREFIX + self.__module__) Line 23: setattr(target, "_logger", _logger) -- 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: 4 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
