Francesco Romani has posted comments on this change. Change subject: Support for timezone reporting ......................................................................
Patch Set 1: (4 comments) The patch could be good enough, but I think there is room for improvement. 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 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? 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? IIRC the LSB guarantees the file presence/readability but I'm not sure 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: fullname = os.path.join(root, name) if os.path.samefile(current, fullname): return _zoneinfo_to_tz(fullname) 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: 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
