Merge lp:~sil/desktopcouch/glib-callback-for-changes into lp:desktopcouch

[Stuart Langridge]

An implementation of glib_callback_for_changes which allows a glib program to attach a callback which is called whenever a change happens in a desktopcouch database.

An implementation of glib_callback_for_changes which allows a glib program to attach a callback which is called whenever a change happens in a desktopcouch database.

Use in a glib program:

def my_callback(data):
    print "a record with id %s was changed" % data["id"]
db = CouchDatabase("whatever")
db.glib_callback_for_changes(my_callback)
gtk.main()

[Eric Casteleijn (thisfred)]

I like this functionality, but would like it better if it weren't a member of CouchDatabase (which I would like to remain agnostic of anything platform specific.) My intuition is that it doesn't need to be, since it makes relatively little use of self. I would probably strive for a function called something like 'add_glib_callback' that takes a database object and a callback function.

In addition I'd like a more generic callback function which knows nothing about glib, which could then be used by the glib one.

I know little of glib, so I don't know how realistic these preferences are.

[Eric Casteleijn (thisfred)]

In addition, I get an import error.

===============================================================================
[ERROR]: desktopcouch.records.tests.test_server.TestCouchDatabase.test_glib_callback_for_changes

Traceback (most recent call last):
Failure: testtools.testresult.real._StringException: Text attachment: traceback
------------
Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/testtools/runtest.py", line 128, in _run_user
    return fn(*args)
  File "/usr/lib/python2.6/dist-packages/testtools/testcase.py", line 368, in _run_test_method
    testMethod()
  File "/home/eric/canonical/desktopcouch/glib-callback-for-changes/desktopcouch/records/tests/test_server.py", line 705, in test_glib_callback_for_changes
    self.database.glib_callback_for_changes(callback)
  File "/home/eric/canonical/desktopcouch/glib-callback-for-changes/desktopcouch/records/server.py", line 121, in glib_callback_for_changes
    from gi.repository import Soup
ImportError: cannot import name Soup

review: Needs Fixing

[Eric Casteleijn (thisfred)]

After installing gir1.0-soup-2.4 (which points to why I'd like this somewhere separate, since I don't want that as a dependency of the library,) the tests pass, but:

== Pylint notices ==
************* Module desktopcouch.records.server
C0301:179: Line too long (81/80)
R0914:108:CouchDatabase.glib_callback_for_changes: Too many local variables (26/15)
E0611:123:CouchDatabase.glib_callback_for_changes: No name 'repository' in module 'gi'
C0111:129:CouchDatabase.glib_callback_for_changes.got_chunk: Missing docstring
W0613:129:CouchDatabase.glib_callback_for_changes.got_chunk: Unused argument 'msg'
C0111:152:CouchDatabase.glib_callback_for_changes.complete: Missing docstring
W0613:152:CouchDatabase.glib_callback_for_changes.complete: Unused argument 'args'
E1103:176:CouchDatabase.glib_callback_for_changes: Instance of 'Http' has no 'oauth_data' member (but some types could not be inferred)
W0612:180:CouchDatabase.glib_callback_for_changes: Unused variable 'fragment'
W0612:180:CouchDatabase.glib_callback_for_changes: Unused variable 'netloc'
W0612:180:CouchDatabase.glib_callback_for_changes: Unused variable 'path'
W0612:125:CouchDatabase.glib_callback_for_changes: Unused variable 'urllib'
W0612:180:CouchDatabase.glib_callback_for_changes: Unused variable 'schema'
W0201:159:CouchDatabase.glib_callback_for_changes: Attribute '_soupsessions' defined outside __init__
************* Module desktopcouch.records.tests.test_server
W0232:699:TestCouchDatabase.test_glib_callback_for_changes.PythonCannotSetNonGlobalOuterScopeVars: Class has no __init__ method
C0111:699:TestCouchDatabase.test_glib_callback_for_changes.PythonCannotSetNonGlobalOuterScopeVars: Missing docstring
C0321:699:TestCouchDatabase.test_glib_callback_for_changes.PythonCannotSetNonGlobalOuterScopeVars: More than one statement on a single line
R0903:699:TestCouchDatabase.test_glib_callback_for_changes.PythonCannotSetNonGlobalOuterScopeVars: Too few public methods (0/2)
W0201:705:TestCouchDatabase.test_glib_callback_for_changes.callback: Attribute 'success' defined outside __init__
C0111:704:TestCouchDatabase.test_glib_callback_for_changes.callback: Missing docstring
W0613:704:TestCouchDatabase.test_glib_callback_for_changes.callback: Unused argument 'data'
C0111:707:TestCouchDatabase.test_glib_callback_for_changes.timeout: Missing docstring
C0111:709:TestCouchDatabase.test_glib_callback_for_changes.insert_record: Missing docstring

[Stuart Langridge (sil)]

You'll note that the glib callback function imports the Python gir stuff whe nit's called, not at the top level in the module, precisely so that using gir is not a dependency *unless you use glib_callback_for_changes*. The apt dependency system isn't set up all that well for this; putting this one function in a package all by itself would be daft. I suggest that the resulting package Suggests python-gir-soup as per http://www.debian.org/doc/debian-policy/ch-relationships.html#s-binarydeps.

The function delves into interior details of CouchDatabase in order to compute a _changes URL. Something external to CouchDatabase should not be using this sort of internal API; it's an implementation detail which we may change.

You're right about the pylint stuff, although I don't understand why I didn't get told the same things when running the tests? What am I doing wrong there?

[Eric Casteleijn (thisfred)]

> You'll note that the glib callback function imports the Python gir stuff whe
> nit's called, not at the top level in the module, precisely so that using gir
> is not a dependency *unless you use glib_callback_for_changes*. The apt
> dependency system isn't set up all that well for this; putting this one
> function in a package all by itself would be daft. I suggest that the
> resulting package Suggests python-gir-soup as per http://www.debian.org/doc
> /debian-policy/ch-relationships.html#s-binarydeps.

Yeah, I really hate having a function that will break as soon as you call it, though. I don't suggest having this in a separate package, but having it as a top level function won't show it in introspection of CouchDatabase, and allows us to move it into desktopcouch.application, where I think the dependency (or suggestion) is less of a problem.

> The function delves into interior details of CouchDatabase in order to compute
> a _changes URL. Something external to CouchDatabase should not be using this
> sort of internal API; it's an implementation detail which we may change.

If so we can reimplement the function, I don't consider it as big a problem as having a broken method on our most central class.

> You're right about the pylint stuff, although I don't understand why I didn't
> get told the same things when running the tests? What am I doing wrong there?

The preferred way to run the tests now is ./runtests.py which takes care of the python path and everything else. It will run lint on the changed files only unless there are no local changes, and then it will lint every file that was changed in the branch, so if you want to lint the whole branch, make sure all your changes are committed.

[lp:~sil/desktopcouch/glib-callback-for-changes]

201. By Stuart Langridge on 2010-11-16

bow and scrape before the almighty god pylint

202. By Stuart Langridge on 2010-11-16

merge from trunk

[Eric Casteleijn (thisfred)]

Looks good now.

review: Approve

[lp:~sil/desktopcouch/glib-callback-for-changes]

203. By Stuart Langridge on 2010-11-16

correct enable message

[Vincenzo Di Somma (vds)]

I might be wrong but in Lucid I don't find a package to satisfy the required dependency and the tests break.

===============================================================================
[ERROR]: desktopcouch.records.tests.test_server.TestCouchDatabase.test_glib_callback_for_changes

Traceback (most recent call last):
Failure: testtools.testresult.real._StringException: Text attachment: traceback
------------
Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/testtools/runtest.py", line 128, in _run_user
    return fn(*args)
  File "/usr/lib/python2.6/dist-packages/testtools/testcase.py", line 368, in _run_test_method
    testMethod()
  File "/home/vds/canonical/desktopcouch/glib-callback-for-changes/desktopcouch/records/tests/test_server.py", line 725, in test_glib_callback_for_changes
    self.database.glib_callback_for_changes(callback)
  File "/home/vds/canonical/desktopcouch/glib-callback-for-changes/desktopcouch/records/server.py", line 126, in glib_callback_for_changes
    from gi.repository import Soup # pylint: disable=E0611
ImportError: No module named gi.repository

review: Needs Fixing

[dobey (dobey)]

Also, please use timeout_add_seconds(X) instead of timeout_add(X * 1000), as it is the preferred way to do timeouts that large.

review: Needs Fixing

Unmerged revisions

203. By Stuart Langridge on 2010-11-16

correct enable message

202. By Stuart Langridge on 2010-11-16

merge from trunk

201. By Stuart Langridge on 2010-11-16

bow and scrape before the almighty god pylint

200. By Stuart Langridge on 2010-11-09

An implementation of glib_callback_for_changes which allows a glib program to attach a callback which is called whenever a change happens in a desktopcouch database

=== modified file 'desktopcouch/records/server.py'
--- desktopcouch/records/server.py	2010-11-10 21:17:11 +0000
+++ desktopcouch/records/server.py	2010-11-16 14:16:13 +0000
@@ -25,7 +25,7 @@
 import uuid
 
 from couchdb import Server
-from couchdb.client import Resource
+from couchdb.client import Resource, uri as couchdburi
 import desktopcouch
 from desktopcouch.records import server_base
 from desktopcouch.platform import find_port
@@ -65,6 +65,9 @@
                  ctx=desktopcouch.local_files.DEFAULT_CONTEXT):
         self.ctx = ctx
         self.server_uri = uri
+
+        self._soupsessions = []
+
         super(CouchDatabase, self).__init__(
                 database, uri, record_factory=record_factory, create=create,
                 server_class=server_class, oauth_tokens=oauth_tokens, ctx=ctx)
@@ -104,3 +107,96 @@
             uri = self.server_uri
         super(CouchDatabase, self)._reconnect(uri=uri)
     # pylint: enable=W0221
+
+    def glib_callback_for_changes(self, callback, # pylint: disable=R0914
+            error_callback=None, since=None): 
+        """Set up a callback to be called, asynchronously, whenever a
+           change is fired by this database's CouchDB _changes feed.
+           Note: this callback requires the glib mainloop, but it does
+           not start it. It is the caller's responsibility to run the glib
+           mainloop.
+           The since parameter is a CouchDB sequence number from which you 
+           wish to monitor changes. The default, None, means to monitor changes
+           from now on. If the caller remembers the last seqno it saw, then
+           it can pass that as since to get all changes since that last seqno
+           sent to the callback as well as any future changes.
+           The callback is called with one argument, a dictionary parsed from
+           the CouchDB changes feed item, containing id, seq, and changes
+           members."""
+        from gi.repository import Soup  # pylint: disable=E0611
+        from oauth import oauth
+        import cgi, json
+        session = Soup.SessionAsync()
+
+        # inner functions to handle changes when they arrive
+        def got_chunk(msg, chunkbuffer): # pylint: disable=W0613
+            """Internal function to handle one line from _changes."""
+            data = ''.join([chr(x) for x in chunkbuffer.get_data()])
+            if not data.strip():
+                # got a heartbeat
+                return
+            try:
+                actual_data = json.loads(data)
+            except ValueError:
+                # weird, we got something that wasn't JSON. Ignore it.
+                if error_callback is not None:
+                    error_callback({
+                        "reason": "Got some data that wasn't JSON",
+                        "error": "could_not_parse_json",
+                        "data": data
+                    })
+                return
+            if "id" not in actual_data or "changes" not in actual_data or \
+               "seq" not in actual_data:
+                if error_callback is not None:
+                    error_callback(actual_data)
+                return
+            callback(actual_data)
+
+        def complete(*args): # pylint: disable=W0613
+            """Internal function called if connection ever completes."""
+            # should never get called, because connection is long-lived
+            # it is called when the loop is ended, though.
+            pass
+
+        # make sure session does not quit when function ends
+        self._soupsessions.append(session)
+
+        # Calculate the changes URI for this database, and oauth-sign it
+        # so that libsoup is able to fetch it
+        params = dict(feed="continuous", 
+            heartbeat=1000 # newline every 10 seconds to keep connection alive
+        )
+        if since is None:
+            params["since"] = self.db.info()["update_seq"]
+        else:
+            params["since"] = since
+        changes_feed_uri = couchdburi( # pylint: disable=W0142
+            self._server.resource.uri, self.db.name, "_changes", 
+            **params)
+        # note: this function does its own oauth signing rather than calling
+        # the similar oauth signing stuff in server_base.
+        oauth_data = \
+            self._server.resource.http.oauth_data # pylint: disable=E1103
+        consumer = oauth.OAuthConsumer(oauth_data["consumer_key"], 
+            oauth_data["consumer_secret"])
+        token = oauth.OAuthToken(oauth_data["token"], 
+            oauth_data["token_secret"])
+        parts = urlparse.urlparse(changes_feed_uri)
+        query = list(parts)[4]
+        querystr_as_dict = dict(cgi.parse_qsl(query))
+        req = oauth.OAuthRequest.from_consumer_and_token(
+            consumer,
+            token,
+            http_method = "GET",
+            http_url = changes_feed_uri,
+            parameters = querystr_as_dict
+        )
+        req.sign_request(oauth.OAuthSignatureMethod_HMAC_SHA1(), consumer, 
+            token)
+        signed_changes_feed_uri = req.to_url()
+
+        msg = Soup.Message.new("GET", signed_changes_feed_uri)
+        session.queue_message(msg, complete, signed_changes_feed_uri)
+        msg.connect("got-chunk", got_chunk)
+

=== modified file 'desktopcouch/records/tests/test_server.py'
--- desktopcouch/records/tests/test_server.py	2010-11-16 13:52:34 +0000
+++ desktopcouch/records/tests/test_server.py	2010-11-16 14:16:13 +0000
@@ -690,3 +690,42 @@
             self.fail()
         except FieldsConflict, e:
             self.assertEqual({('field1',): (22, 11)}, e.conflicts)
+
+    def test_glib_callback_for_changes(self):
+        """Test glib_callback_for_changes method"""
+        import gobject
+        
+        # set var on namespace class as per example 7 at 
+        # http://www.saltycrane.com/blog/2008/01/python-variable-scope-notes/
+        # because Python 2 can't set outer-but-non-global variables
+        class PythonCannotSetNonGlobalOuterScopeVars: # pylint: disable=R0903
+            """Internal class to provide namespace for outer variable."""
+            def __init__(self):
+                """Pointless init method just to make pylint happy."""
+                self.success = False
+        p = PythonCannotSetNonGlobalOuterScopeVars()
+        
+        loop = gobject.MainLoop()
+        def callback(data): # pylint: disable=W0613
+            """Callback handler to confirm that a callback occurs."""
+            p.success = True
+            loop.quit()
+        def timeout():
+            """Timeout handler to catch failure to call the callback."""
+            loop.quit()
+        def insert_record():
+            """Trigger a record insertion (and thus a callback)."""
+            self.database.put_record(Record({
+                "first": "value1", "second": "value2",
+                "record_type": "callbacktest.com"}))
+
+        # Set up the callback, then in 1s insert a record; if the callback
+        # works, then that insert will call the callback; if it doesn't,
+        # then the timeout will be called. 
+        
+        self.database.glib_callback_for_changes(callback)
+        gobject.timeout_add(1000, insert_record)
+        gobject.timeout_add(3000, timeout)
+        loop.run()
+        self.assertTrue(p.success)
+