Vinzenz Feenstra has posted comments on this change.

Change subject: Implementation of logind based session locking
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.ovirt.org/#/c/24628/1/ovirt-guest-agent/LockActiveSession.py
File ovirt-guest-agent/LockActiveSession.py:

Line 32: )
> Just wondering: can this props cached or lazily created and kept?
This whole script is only executed once in a while, so this doesn't really need 
performance gains, I went for a straightforward implementation. This is a 
script which is executed in the context of root when the guest agent gets the 
instruction to lock the session


Line 64:                 dbus_interface='org.freedesktop.ConsoleKit.Session')
Line 65:             if s.IsActive():
Line 66:                 session = s
Line 67:                 break
Line 68:     except:
> Not a big fan of those
My normally either, but I catching anything to continue execution on any 
failure.
Line 69:         pass
Line 70:     return session
Line 71: 
Line 72: 


Line 89:                 break
Line 90:             session = None
Line 91:     except:
Line 92:         pass
Line 93:     return session
> Would it make sense to return the valid session early in the for loop and r
I don't know, I try to avoid multiple exit points
Line 94: 
Line 95: 
Line 96: def GetActiveSession():
Line 97:     bus = dbus.SystemBus()


-- 
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: 1
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

Reply via email to