msmith - in flumotion/branches/manager-cleanup-1: . flumotion/manager

flumotion-commit at lists.fluendo.com flumotion-commit at lists.fluendo.com
Fri May 25 12:47:09 CEST 2007


Author: msmith
Date: Fri May 25 12:47:04 2007
New Revision: 5036

Modified:
   flumotion/branches/manager-cleanup-1/ChangeLog
   flumotion/branches/manager-cleanup-1/flumotion/manager/admin.py
   flumotion/branches/manager-cleanup-1/flumotion/manager/base.py
   flumotion/branches/manager-cleanup-1/flumotion/manager/component.py
   flumotion/branches/manager-cleanup-1/flumotion/manager/manager.py
   flumotion/branches/manager-cleanup-1/flumotion/manager/worker.py
Log:
        * flumotion/manager/admin.py:
        * flumotion/manager/base.py:
        * flumotion/manager/component.py:
        * flumotion/manager/manager.py:
        * flumotion/manager/worker.py:
        Branched to hack on manager internals.
        Cleanup of login, particularly to avoid callLater nastiness and race
        conditions inherent in that.



Modified: flumotion/branches/manager-cleanup-1/ChangeLog
==============================================================================
--- flumotion/branches/manager-cleanup-1/ChangeLog	(original)
+++ flumotion/branches/manager-cleanup-1/ChangeLog	Fri May 25 12:47:04 2007
@@ -1,3 +1,14 @@
+2007-05-25  Michael Smith <msmith at fluendo.com>
+
+	* flumotion/manager/admin.py:
+	* flumotion/manager/base.py:
+	* flumotion/manager/component.py:
+	* flumotion/manager/manager.py:
+	* flumotion/manager/worker.py:
+	Branched to hack on manager internals.
+	Cleanup of login, particularly to avoid callLater nastiness and race
+	conditions inherent in that. 
+
 2007-05-24  Michael Smith <msmith at fluendo.com>
 
 	* configure.ac:

Modified: flumotion/branches/manager-cleanup-1/flumotion/manager/admin.py
==============================================================================
--- flumotion/branches/manager-cleanup-1/flumotion/manager/admin.py	(original)
+++ flumotion/branches/manager-cleanup-1/flumotion/manager/admin.py	Fri May 25 12:47:04 2007
@@ -53,13 +53,13 @@
     logCategory = 'admin-avatar'
        
     # override base methods
-    def attached(self, mind):
+    def mindAttached(self, mind):
         self.info('admin client "%s" logged in' % self.avatarId)
-        base.ManagerAvatar.attached(self, mind)
+        return base.ManagerAvatar.mindAttached(self, mind)
 
-    def detached(self, mind):
+    def mindDetached(self, mind):
         self.info('admin client "%s" logged out' % self.avatarId)
-        base.ManagerAvatar.detached(self, mind)
+        base.ManagerAvatar.mindDetached(self, mind)
 
     # FIXME: instead of doing this, give a RemoteCache of the heaven state ?
     def getComponentStates(self):

Modified: flumotion/branches/manager-cleanup-1/flumotion/manager/base.py
==============================================================================
--- flumotion/branches/manager-cleanup-1/flumotion/manager/base.py	(original)
+++ flumotion/branches/manager-cleanup-1/flumotion/manager/base.py	Fri May 25 12:47:04 2007
@@ -174,12 +174,13 @@
              f, log.getFailureMessage(f)))
         return f
 
-    def attached(self, mind):
+    def mindAttached(self, mind):
         """
         Tell the avatar that the given mind has been attached.
         This gives the avatar a way to call remotely to the client that
         requested this avatar.
-        This is scheduled by the portal after the client has logged in.
+        This is scheduled by the portal during client login (after it is 
+        authenticated, but before the remote login method returns)
 
         @type mind: L{twisted.spread.pb.RemoteReference}
         """
@@ -196,7 +197,9 @@
         # Now we have a remote reference, so start checking pings.
         self.startPingChecking(self.timeoutDisconnect)
 
-    def detached(self, mind):
+        return defer.succeed(None)
+
+    def mindDetached(self, mind):
         """
         Tell the avatar that the peer's client referenced by the mind
         has detached.
@@ -411,7 +414,7 @@
         self.avatars = {} # avatarId -> avatar
        
     ### ManagerHeaven methods
-    def createAvatar(self, avatarId, remoteIdentity):
+    def createAvatar(self, avatarId, remoteIdentity, mind):
         """
         Create a new avatar and manage it.
 
@@ -420,9 +423,11 @@
         @param remoteIdentity: the manager-side representation of the
                                remote identity
         @type  remoteIdentity: L{flumotion.common.identity.RemoteIdentity}
+        @param mind:           Remote reference to the client this avatar is 
+                               being created for.
+        @type  mind:           L{twisted.spread.pb.RemoteReference}
 
-        @returns: a new avatar for the client
-        @rtype:   L{flumotion.manager.base.ManagerAvatar}
+        @returns: a deferred that will fire with a new avatar for the client
         """
         self.debug('creating new Avatar with name %s' % avatarId)
         if self.avatars.has_key(avatarId):
@@ -431,19 +436,36 @@
             raise errors.AlreadyConnectedError(avatarId)
 
         avatar = self.avatarClass(self, avatarId, remoteIdentity)
-        
-        self.avatars[avatarId] = avatar
-        return avatar
 
-    def removeAvatar(self, avatarId):
+        def mindAttachComplete(res):
+            # TODO: What happens if two things concurrently log in as the same
+            # avatarId?
+            self.avatars[avatarId] = avatar
+            return avatar
+
+        d = avatar.mindAttached(mind)
+        d.addCallback(mindAttachComplete)
+        return d
+
+    def removeAvatar(self, avatarId, mind):
         """
         Stop managing the given avatar.
 
         @param avatarId: id of the avatar to remove
         @type  avatarId: str
+        @param mind:           Remote reference to the client that is being 
+                               removed.
+        @type  mind:           L{twisted.spread.pb.RemoteReference}
         """
+
         self.debug('removing Avatar with id %s' % avatarId)
-        del self.avatars[avatarId]
+        if avatarId in self.avatars:
+            avatar = self.avatars[avatarId]
+            avatar.mindDetached(mind)
+
+            del self.avatars[avatarId]
+        else:
+            self.warning("Not found in heaven!")
         
     def getAvatar(self, avatarId):
         """

Modified: flumotion/branches/manager-cleanup-1/flumotion/manager/component.py
==============================================================================
--- flumotion/branches/manager-cleanup-1/flumotion/manager/component.py	(original)
+++ flumotion/branches/manager-cleanup-1/flumotion/manager/component.py	Fri May 25 12:47:04 2007
@@ -151,12 +151,19 @@
         print "Ignore the following Traceback line, issue in Twisted"
         return failure
 
-    def attached(self, mind):
+    def mindAttached(self, mind):
         # doc in base class
         self.info('component "%s" logged in' % self.avatarId)
-        base.ManagerAvatar.attached(self, mind) # sets self.mind
-        
-        d = self.vishnu.componentAttached(self)
+        base.ManagerAvatar.mindAttached(self, mind) # sets self.mind
+
+        def gotConfigAndState(res):
+            (_success1, conf), (_success2, jobState) = res
+            self.vishnu.componentAttached(self, conf, jobState)
+        
+        d = defer.DeferredList([self.mindCallRemote('getConfig'),
+                                self.mindCallRemote('getState')],
+                               fireOnOneErrback=True)
+        d.addCallback(gotConfigAndState)
 
         def checkInitialMood(_):
             # If we're already set to happy, ensure that we trigger our 
@@ -175,7 +182,7 @@
         d.addCallback(lambda _: self.vishnu.registerComponent(self))
         return d
 
-    def detached(self, mind):
+    def mindDetached(self, mind):
         # doc in base class
         self.vishnu.unregisterComponent(self)
         self.heaven.unregisterComponent(self)
@@ -199,7 +206,7 @@
                 self._setMood(moods.lost)
 
         self.vishnu.componentDetached(self)
-        base.ManagerAvatar.detached(self, mind)
+        base.ManagerAvatar.mindDetached(self, mind)
 
         self.cleanup() # callback done at end
  

Modified: flumotion/branches/manager-cleanup-1/flumotion/manager/manager.py
==============================================================================
--- flumotion/branches/manager-cleanup-1/flumotion/manager/manager.py	(original)
+++ flumotion/branches/manager-cleanup-1/flumotion/manager/manager.py	Fri May 25 12:47:04 2007
@@ -95,7 +95,6 @@
         # FIXME: Is passing a callable to a constructor offending anyone
         # else's sense of aesthetics ?
         self._interfaceHeavens = {} # interface -> heaven
-        self._avatarHeavens = {} # avatarId -> heaven
         self._computeIdentity = computeIdentity
         
     ### IRealm methods
@@ -116,12 +115,8 @@
             # uses these kwargs to set its own info. so don't change
             # these args or their order or you will break your test
             # suite.
-            def cleanup(avatarId=avatarId, avatar=avatar, mind=mind):
-                self.removeAvatar(avatarId, avatar, mind)
-            # schedule a perspective attached for after this function
-            # FIXME: there needs to be a way to not have to do a callLater
-            # blindly so cleanup can be guaranteed
-            reactor.callLater(0, avatar.attached, mind)
+            def cleanup(avatarId=avatarId, mind=mind):
+                self.removeAvatar(avatarId, mind)
             return (pb.IPerspective, avatar, cleanup)
         def got_error(failure):
             # If we failed for some reason, we want to drop the connection.
@@ -133,25 +128,21 @@
             return failure
 
         host = common.addressGetHost(mind.broker.transport.getPeer())
-        d = self.createAvatarFor(avatarId, keycard, host, ifaces)
+        d = self.createAvatarFor(avatarId, keycard, host, ifaces, mind)
         d.addCallbacks(got_avatar, got_error)
         return d
 
     ### our methods
 
-    def removeAvatar(self, avatarId, avatar, mind):
+    def removeAvatar(self, avatarId, mind):
         """
         Remove an avatar because it logged out of the manager.
         
         This function is registered by requestAvatar.
         """
-        heaven = self._avatarHeavens[avatarId]
-        del self._avatarHeavens[avatarId]
-        
-        avatar.detached(mind)
-        heaven.removeAvatar(avatarId)
+        heaven.removeAvatar(avatarId, mind)
 
-    def createAvatarFor(self, avatarId, keycard, remoteHost, ifaces):
+    def createAvatarFor(self, avatarId, keycard, remoteHost, ifaces, mind):
         """
         Create an avatar from the heaven implementing the given interface.
 
@@ -164,6 +155,9 @@
         @type  ifaces:     tuple of interfaces linked to heaven
         @param ifaces:     a list of heaven interfaces to get avatar from,
                            including pb.IPerspective
+        @type  mind:       L{twisted.spread.pb.RemoteReference}
+        @param mind:       Remote reference to the client we're creating an
+                           avatar for.
 
         @returns:          a deferred that will fire an avatar from
                            the heaven managing the given interface.
@@ -172,11 +166,7 @@
             for iface in ifaces:
                 heaven = self._interfaceHeavens.get(iface, None)
                 if heaven:
-                    avatar = heaven.createAvatar(avatarId, identity)
-                    self.debug('Created avatar %r for identity %r' % (
-                        avatar, identity))
-                    self._avatarHeavens[avatarId] = heaven
-                    return avatar
+                    return heaven.createAvatar(avatarId, identity, mind)
             raise errors.NoPerspectiveError("%s requesting iface %r",
                                             avatarId, ifaces)
             
@@ -837,7 +827,7 @@
         self.debug('vishnu.workerDetached(): id %s' % workerId)
         self._depgraph.setWorkerStopped(workerId)
 
-    def _getComponentState(self, deferredListResult, avatar):
+    def componentAttached(self, avatar, conf, jobState):
         # a component just logged in with good credentials. we fetched
         # its config and job state. now there are two possibilities:
         #  (1) we were waiting for such a component to start. There is a
@@ -914,7 +904,6 @@
             self._componentMappers[m.state] = m
             self._componentMappers[m.id] = m
 
-        (_success1, conf), (_success2, jobState) = deferredListResult
         m = self.getComponentMapper(avatar.avatarId)
 
         if m:
@@ -940,15 +929,6 @@
         m.jobState = jobState
         self._componentMappers[jobState] = m
 
-    def componentAttached(self, componentAvatar):
-        # called when a component logs in and gets a component avatar created
-        self.debug("%s component attached", componentAvatar.avatarId)
-        d = defer.DeferredList([componentAvatar.mindCallRemote('getConfig'),
-                                componentAvatar.mindCallRemote('getState')],
-                               fireOnOneErrback=True)
-        d.addCallback(self._getComponentState, componentAvatar)
-        return d
-
     def componentDetached(self, componentAvatar):
         # called when the component has detached
         self.debug("%s component detached", componentAvatar.avatarId)

Modified: flumotion/branches/manager-cleanup-1/flumotion/manager/worker.py
==============================================================================
--- flumotion/branches/manager-cleanup-1/flumotion/manager/worker.py	(original)
+++ flumotion/branches/manager-cleanup-1/flumotion/manager/worker.py	Fri May 25 12:47:04 2007
@@ -48,10 +48,10 @@
     def getName(self):
         return self.avatarId
 
-    def attached(self, mind):
+    def mindAttached(self, mind):
         # doc in base class
         self.info('worker "%s" logged in' % self.getName())
-        base.ManagerAvatar.attached(self, mind)
+        base.ManagerAvatar.mindAttached(self, mind)
 
         self.debug('MANAGER -> WORKER: getFeedServerPort()')
         d = self.mindCallRemote('getFeedServerPort')
@@ -70,9 +70,10 @@
 
         self.heaven.workerAttached(self)
         self.vishnu.workerAttached(self)
-    attached = defer_generator_method(attached)
 
-    def detached(self, mind):
+    mindAttached = defer_generator_method(mindAttached)
+
+    def mindDetached(self, mind):
         # doc in base class
         self.info('worker "%s" logged out' % self.getName())
         base.ManagerAvatar.detached(self, mind)


More information about the flumotion-commit mailing list