Vinzenz Feenstra has posted comments on this change. Change subject: Support for timezone reporting ......................................................................
Patch Set 1: (4 comments) http://gerrit.ovirt.org/#/c/28013/1/ovirt-guest-agent/OVirtAgentLogic.py File ovirt-guest-agent/OVirtAgentLogic.py: Line 153: except NotImplementedError: Line 154: return -1 Line 155: Line 156: def getTimezone(self): Line 157: import timezone > why here? Not a big deal, mostly curious honestly, I don't know, there's really no reason, I will move it up. Line 158: return timezone.get_name() Line 159: Line 160: Line 161: class AgentLogicBase: http://gerrit.ovirt.org/#/c/28013/1/ovirt-guest-agent/timezone.py File ovirt-guest-agent/timezone.py: Line 22: import os.path Line 23: import platform Line 24: Line 25: Line 26: _IS_WINDOWS = platform.system() in ['Windows', 'Microsoft'] > tuple? yeah, probably worth it Line 27: Line 28: Line 29: def get_name_windows(): Line 30: try: Line 39: return "" Line 40: Line 41: Line 42: def _read_etc_timezone(): Line 43: f = open('/etc/timezone', 'r') > can this (and the one similar below) fail? These are internal functions called when the check have been done already. The only thing what could fail is that the file won't be readable for me but that would break more than just my code, therefore I don't see a problem here. The calls should solely be called by get_name_linux and that checks before that the file exists, same for /etc/sysconfig/clock And no, /etc/timezone is only available on debianoid systems Line 44: result = f.read().strip() Line 45: f.close() Line 46: return result Line 47: Line 81: for root, dirs, names in os.walk(location): Line 82: for name in names: Line 83: fullname = os.path.join(root, name) Line 84: if current == _hash_file(fullname): Line 85: return _zoneinfo_to_tz(fullname) > I think we can use os.path.samefile here: no, because it won't be the same file, it would be a copy of it Line 86: return None Line 87: Line 88: Line 89: def get_name(): -- To view, visit http://gerrit.ovirt.org/28013 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I94c1429f40448a262d0d72fcc6dba2b4b59340c2 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-guest-agent Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@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
