Vinzenz Feenstra has posted comments on this change. Change subject: Implementation of logind based session locking ......................................................................
Patch Set 3: (2 comments) http://gerrit.ovirt.org/#/c/24628/3/ovirt-guest-agent/LockActiveSession.py File ovirt-guest-agent/LockActiveSession.py: Line 59: Line 60: def GetInterfaceByName(bus, service, name, isSub): Line 61: path = '/org/freedesktop/' + service Line 62: if isSub: Line 63: path += '/' + name > As matter as personal taste, I'd just used an optional prefix like And it wouldn't work, because name is passed through to GetInterface The thing is, that for getting the Manager on login1 the path is this: /org/freedesktop/login1 And for ConsoleKit this: /org/freedesktop/ConsoleKit/Manager But then I need org.freedesktop.ConsoleKit for the object and org.freedesktop.ConsoleKit.Manager for the interface So just basing it on name wouldn't work with 'GetInterfaceByName', this is why there's isSub Line 64: return GetInterface(bus, service, name, path) Line 65: Line 66: Line 67: def GetSession(bus, service, managerIsSub, wrapSession, wrapManager): Line 81: Line 82: def ManagerWrapper(manager): Line 83: def f(): Line 84: return [x[4] for x in manager.ListSessions()] Line 85: manager.GetSessions = f > I think f() should be a staticmethod. Or maybe we can extract GetSessions: I am not sure why f() should be a static method? I don't really get it. And this is a DBus object, that method actually does not exist. Anyway the approach of try except is good, and I will use that. However it won't be an AttributeError but a dbus.DBusException Line 86: return manager Line 87: Line 88: Line 89: def GetActiveSession(): -- To view, visit http://gerrit.ovirt.org/24628 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iafc4b88d9c508647e1d09407556359fbefbba34c Gerrit-PatchSet: 3 Gerrit-Project: ovirt-guest-agent Gerrit-Branch: master Gerrit-Owner: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
