Merge lp:~terrycojones/tickery/update-to-latest-txrdq-806699 into lp:tickery

[Terry Jones]

A bunch of small changes to make Tickery work with the latest txRDQ.

In running this I found (and fixed) a couple of other problems that
make no sense to me. The JSONRPC service receives str screennames (not
unicode) and passes them to the Tickery Twitter methods (which use
txFluidDB) which complain that the screennames are not unicode. So I
converted them. I don't know how the old code was managing to run,
but presumably something was different. I don't know why the JSONRPC
lib isn't giving us unicode screennames. Weird.

I also added a requirements-local.txt which I'd like you to ignore for
now. It's just a record of things we will need to install locally
when we move to having a Fluidinfo Tickery development/deploy project
as well as a neutral (non-Fluidinfo specific) Tickery release.

[Jamu Kakar (jkakar)]

[1]

I've reviewed the changes and I think I vaguely understand them (but
definitely not deeply). Anyway, before learning more I can't even
build Tickery:

$ make
fab build_pyjs
[localhost] run: cd tickery/www && ../../pyjamas/bin/pyjsbuild index.py

Fatal error: local() encountered an error (return code 127) while executing 'cd tickery/www && ../../pyjamas/bin/pyjsbuild index.py'

Aborting.
make: *** [all] Error 1

Do you know what's going on here?

review: Needs Information

[Eric Seidel (gridaphobe)]

> $ make
> fab build_pyjs
> [localhost] run: cd tickery/www && ../../pyjamas/bin/pyjsbuild index.py
>
> Fatal error: local() encountered an error (return code 127) while executing
> 'cd tickery/www && ../../pyjamas/bin/pyjsbuild index.py'
>
> Aborting.
> make: *** [all] Error 1
>
> Do you know what's going on here?

You need to run `fab install_pyjs` to install pyjamas before building
the app. This should be documented or just included in the default target.
Also, there's a silly typo in the Makefile, unless debub is an actual word :P

[Terry Jones (terrycojones)]

Hi Jamu (and thanks Eric!)

The setup is a bit better in the fix-fab-deploy-798485 branch (see the
Makefile in there). We need to separate the Tickery code from the
Fluidinfo project to build and deploy our own running instance. I'm
waiting to do that until more of the merge proposals are accepted
(mainly 798485). I don't want to document a deploy procedure in the
public branch that is only relevant to us. There is a doc dir, which
is docs for the public (probably not 100%) accurate.

Anyway, as Eric says, if you do fab install_pyjs (which will be done
via make when 798485 lands), you'll have a top-level pyjamas dir. You
should then

$ virtualenv env
$ . env/bin/activate
$ pip install --update -r requirements.txt

and at that point if you run just twistd, you should see tickery
listed as a plugin. So,

$ twistd tickery

then visit http://localhost:6969

Let me know if you have a problem. Thanks!

[Esteve Fernandez (esteve)]

+1 LGTM!

review: Approve

[Jamu Kakar (jkakar)]

> Anyway, as Eric says, if you do fab install_pyjs (which will be done
> via make when 798485 lands), you'll have a top-level pyjamas dir. You
> should then
>
> $ virtualenv env
> $ . env/bin/activate
> $ pip install --update -r requirements.txt

Thanks, this worked. I had to remove '--update' from the command-line
options. I guess this is a difference in the version of PIP on Ubuntu
11.04.

> and at that point if you run just twistd, you should see tickery
> listed as a plugin. So,
>
> $ twistd tickery
>
> then visit http://localhost:6969
>
> Let me know if you have a problem. Thanks!

It still doesn't work, I guess I need a Twitter API key (or
something):

$ twistd tickery
Unhandled Error
Traceback (most recent call last):
  File "/home/jkakar/src/projects/twisted/trunk/twisted/application/app.py", line 595, in parseOptions
    usage.Options.parseOptions(self, options)
  File "/home/jkakar/src/projects/twisted/trunk/twisted/python/usage.py", line 226, in parseOptions
    for (cmd, short, parser, doc) in self.subCommands:
  File "/home/jkakar/src/projects/twisted/trunk/twisted/application/app.py", line 607, in subCommands
    for plug in plugins:
  File "/home/jkakar/src/projects/twisted/trunk/twisted/plugin.py", line 209, in getPlugins
    allDropins = getCache(package)
--- <exception caught here> ---
  File "/home/jkakar/src/projects/twisted/trunk/twisted/plugin.py", line 167, in getCache
    provider = pluginModule.load()
  File "/home/jkakar/src/projects/twisted/trunk/twisted/python/modules.py", line 383, in load
    return self.pathEntry.pythonPath.moduleLoader(self.name)
  File "/home/jkakar/src/projects/twisted/trunk/twisted/python/reflect.py", line 464, in namedAny
    topLevelPackage = _importAndCheckStack(trialname)
  File "/home/jkakar/src/projects/twisted/trunk/twisted/python/reflect.py", line 400, in _importAndCheckStack
    return __import__(importName)
  File "/home/jkakar/src/fluidinfo/tickery/reviews/update-to-latest-txrdq-806699/twisted/plugins/tickery_service.py", line 25, in <module>
    from tickery.cache import TickeryCache
  File "/home/jkakar/src/fluidinfo/tickery/reviews/update-to-latest-txrdq-806699/tickery/cache.py", line 23, in <module>
    from tickery.usercache import TwitterUserCache
  File "/home/jkakar/src/fluidinfo/tickery/reviews/update-to-latest-txrdq-806699/tickery/usercache.py", line 18, in <module>
    from tickery import twitter
  File "/home/jkakar/src/fluidinfo/tickery/reviews/update-to-latest-txrdq-806699/tickery/twitter.py", line 27, in <module>
    from tickery import oauth, consumer, signature, version, error as terror
  File "/home/jkakar/src/fluidinfo/tickery/reviews/update-to-latest-txrdq-806699/tickery/consumer.py", line 36, in <module>
    consumer = TickeryConsumer().consumer
  File "/home/jkakar/src/fluidinfo/tickery/reviews/update-to-latest-txrdq-806699/tickery/consumer.py", line 28, in __init__
    TICKERY_CONSUMER_KEY_ENV_VAR)
exceptions.Exception: Please set 'TICKERY_CONSUMER_KEY' in your environment.

Anyway, the changes in the branch look good to me, +1!

review: Approve

[Esteve Fernandez (esteve)]

> > $ virtualenv env
> > $ . env/bin/activate
> > $ pip install --update -r requirements.txt
>
> Thanks, this worked. I had to remove '--update' from the command-line
> options. I guess this is a difference in the version of PIP on Ubuntu
> 11.04.

Virtualenv installs the latest version of PIP onto the new environment, overriding the system one. However, the correct syntax is --upgrade, not --update. pip install --upgrade (or .-U) -r requirements.txt should work.

=== added file 'requirements-local.txt'
--- requirements-local.txt	1970-01-01 00:00:00 +0000
+++ requirements-local.txt	2011-07-11 00:54:27 +0000
@@ -0,0 +1,2 @@
+simplejson
+ply

=== modified file 'tickery/adder.py'
--- tickery/adder.py	2011-06-16 16:10:07 +0000
+++ tickery/adder.py	2011-07-11 00:54:27 +0000
@@ -12,8 +12,6 @@
 # implied.  See the License for the specific language governing
 # permissions and limitations under the License.
 
-import time
-
 from operator import attrgetter
 
 from twisted.internet import defer
@@ -43,8 +41,6 @@
 
     def reset(self):
         self.state = 'queued'
-        self.queuedAt = time.time()
-        self.failureCount = 0
 
     def setState(self, newState):
         currentState = self.state
@@ -58,21 +54,6 @@
                     log.msg('Logic error? %r is already in state %r.' %
                             (screenname, newState))
                 else:
-                    if newState == 'queued':
-                        self.queuedAt = time.time()
-                    elif newState == 'underway':
-                        now = time.time()
-                        log.msg('User %r was queued for %.2f seconds.' %
-                                (screenname, now - self.queuedAt))
-                        self.underwayAt = now
-                    elif newState == 'added':
-                        log.msg('User %r was underway for %.2f seconds.' %
-                                (screenname, time.time() - self.underwayAt))
-                    elif newState == 'failed':
-                        self.failureCount += 1
-                        log.msg('%r failed. Failure count = %d' %
-                                (screenname, self.failureCount))
-
                     log.msg('Adder: %r state change: %r -> %r.' %
                             (screenname, currentState, newState))
                     self.state = newState
@@ -119,7 +100,8 @@
                     log.msg('Restoring %r (previous state %r)' %
                             (user.screenname, user.state))
                     user.reset()
-                    self.rdq.put(user)
+                    d = self.rdq.put(user)
+                    d.addErrback(self._reportCancelled, user.screenname)
                     self.clean = False
             else:
                 log.msg('Not restoring formerly queued names.')
@@ -147,7 +129,8 @@
             self.users[screennameLower] = user
         log.msg('Adding screenname %r to request queue.' % screenname)
         self.clean = False
-        self.rdq.put(user)
+        d = self.rdq.put(user)
+        d.addErrback(self._reportCancelled, screenname)
 
     def _addUser(self, user):
         def _added(result):
@@ -179,28 +162,18 @@
         except KeyError:
             raise Exception('Cannot cancel unknown user %r.' % screenname)
         else:
-            if user.state == 'underway':
-                for item in self.rdq.underway():
-                    if item.job.screenname == screenname:
-                        log.msg('Cancelling underway %r addition.' %
-                                screenname)
-                        item.cancel()
-                        user.setState('canceled')
-                        break
-                else:
-                    raise Exception('Could not find %r in underway list.' %
-                                    screenname)
-            elif user.state == 'queued':
-                queued = self.rdq.pending()
-                for i, u in enumerate(queued):
+            if user.state == 'underway' or user.state == 'queued':
+                for job in self.rdq.underway() + self.rdq.pending():
+                    u = job.jobarg
                     if u.screenname == screenname:
-                        del queued[i]
+                        log.msg('Cancelling %s %r addition.' %
+                                (user.state, screenname))
+                        job.cancel()
                         user.setState('canceled')
-                        log.msg('Canceled queued %r addition.' % screenname)
                         break
                 else:
-                    raise Exception('Could not find %r in queued list.' %
-                                    screenname)
+                    raise Exception('Could not find %r in underway '
+                                    'or pending lists.' % screenname)
             else:
                 user.setState('canceled')
 
@@ -217,9 +190,9 @@
 
     def statusSummary(self, screennames):
         position = {}
-        for i, user in enumerate(self.rdq.pending()):
+        for i, user in enumerate(
+            [job.jobarg for job in self.rdq.pending()]):
             position[user.screenname.lower()] = i
-        log.msg('position dict is %r' % (position,))
         queued = []
         underway = []
         added = []
@@ -273,3 +246,10 @@
             log.msg('Pending user additions canceled: %r' %
                     [p.screenname for p in pending])
         super(AdderCache, self).close()
+
+    def _reportCancelled(self, fail, screenname):
+        """
+        A user addition was cancelled. Log it and absorb the failure.
+        """
+        fail.trap(defer.CancelledError)
+        log.msg('Addition of user %r cancelled.' % screenname)

=== modified file 'tickery/cache.py'
--- tickery/cache.py	2011-06-16 16:10:07 +0000
+++ tickery/cache.py	2011-07-11 00:54:27 +0000
@@ -18,7 +18,7 @@
 from twisted.internet import defer
 from twisted.application import service
 
-from txrdq.rdq import DeferredPool
+from txrdq.pool import DeferredPool
 
 from tickery.usercache import TwitterUserCache
 from tickery.querycache import QueryCache
@@ -67,7 +67,7 @@
     @defer.inlineCallbacks
     def stopService(self):
         yield self.adderCache.close()
-        yield self.extraTwitterTagsPool.deferUntilEmpty()
+        yield self.extraTwitterTagsPool.notifyWhenEmpty()
         self.userCache.close()
         self.oidUidScreennameCache.close()
         self.queryCache.close()

=== modified file 'tickery/service.py'
--- tickery/service.py	2011-06-21 23:56:18 +0000
+++ tickery/service.py	2011-07-11 00:54:27 +0000
@@ -66,11 +66,20 @@
                 'users': users}}
 
     def jsonrpc_friendOf(self, screenname1, screenname2):
-        # Return True if screenname1 follows screenname2 (i.e., screenname2
-        # is a friend of screenname1).
+        """
+        Does screenname1 follow screenname2?
+
+        @param screenname1: A C{str} Twitter screen name.
+
+        @param screenname2: A C{str} Twitter screen name.
+
+        @return: a C{Deferred} that fires with C{True} if screenname1 follows
+        screenname2 (i.e., screenname2 is a friend of screenname1) and with
+        C{False} otherwise.
+        """
         log.msg('FriendOf %r and %r.' % (screenname1, screenname2))
         d = ftwitter.friendOf(self.cache, self.endpoint,
-                              screenname1, screenname2)
+                              unicode(screenname1), unicode(screenname2))
         d.addCallback(lambda result: {'result': result})
         d.addErrback(log.err)
         return d
@@ -331,22 +340,23 @@
         return {'result': True}
 
     def jsonrpc_getQueued(self):
-        pending = self.cache.adderCache.rdq.pending()
-        result = [[u.screenname, u.nFriends, time.ctime(u.queuedAt)]
-                  for u in pending]
+        pendingJobs = self.cache.adderCache.rdq.pending()
+        result = [[pj.jobarg.screenname, pj.jobarg.nFriends,
+                   time.ctime(pj.queuedTime)] for pj in pendingJobs]
         return {'result': result}
 
     def jsonrpc_getUnderway(self):
-        underway = self.cache.adderCache.rdq.underway()
+        underwayJobs = self.cache.adderCache.rdq.underway()
         result = []
-        for u in underway:
-            pct = 100.0 * float(u.job.workDone) / float(u.job.workToDo)
+        for job in underwayJobs:
+            user = job.jobarg
+            pct = 100.0 * float(user.workDone) / float(user.workToDo)
             result.append([
-                u.job.screenname,
-                u.job.nFriends,
+                user.screenname,
+                user.nFriends,
                 pct,
-                time.ctime(u.job.queuedAt),
-                time.ctime(u.startTime)]
+                time.ctime(job.queuedTime),
+                time.ctime(job.startTime)]
                 )
         return {'result': result}
 
@@ -395,7 +405,7 @@
                 'result': True,
                 }
 
-        d = ftwitter.directAddUser(self.cache, screenname)
+        d = ftwitter.directAddUser(self.cache, unicode(screenname))
         d.addCallbacks(_ok, _err)
         return d
 
@@ -412,7 +422,8 @@
                     'result': True,
                     }
 
-        d = ftwitter.bulkAddUsers(self.cache, screennames.split())
+        d = ftwitter.bulkAddUsers(
+            self.cache, map(unicode, screennames.split()))
         d.addCallback(_ret)
         return d