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

Proposed by Stuart Langridge on 2010-11-09
Status: Work in progress
Proposed branch: lp:~sil/desktopcouch/glib-callback-for-changes
Merge into: lp:desktopcouch
Diff against target: 166 lines (+136/-1)
2 files modified
desktopcouch/records/server.py (+97/-1)
desktopcouch/records/tests/test_server.py (+39/-0)
To merge this branch: bzr merge lp:~sil/desktopcouch/glib-callback-for-changes
Reviewer Review Type Date Requested Status
dobey (community) Needs Fixing on 2010-11-17
Vincenzo Di Somma (community) Needs Fixing on 2010-11-17
Eric Casteleijn (community) 2010-11-09 Approve on 2010-11-16
Review via email: mp+40415@code.launchpad.net

Commit message

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.

Description of the change

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()

To post a comment you must log in.
Eric Casteleijn (thisfred) wrote :

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) wrote :

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) wrote :

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) wrote :

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) wrote :

> 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.

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) wrote :

Looks good now.

review: Approve
203. By Stuart Langridge on 2010-11-16

correct enable message

Vincenzo Di Somma (vds) wrote :

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) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'desktopcouch/records/server.py'
2--- desktopcouch/records/server.py 2010-11-10 21:17:11 +0000
3+++ desktopcouch/records/server.py 2010-11-16 14:16:13 +0000
4@@ -25,7 +25,7 @@
5 import uuid
6
7 from couchdb import Server
8-from couchdb.client import Resource
9+from couchdb.client import Resource, uri as couchdburi
10 import desktopcouch
11 from desktopcouch.records import server_base
12 from desktopcouch.platform import find_port
13@@ -65,6 +65,9 @@
14 ctx=desktopcouch.local_files.DEFAULT_CONTEXT):
15 self.ctx = ctx
16 self.server_uri = uri
17+
18+ self._soupsessions = []
19+
20 super(CouchDatabase, self).__init__(
21 database, uri, record_factory=record_factory, create=create,
22 server_class=server_class, oauth_tokens=oauth_tokens, ctx=ctx)
23@@ -104,3 +107,96 @@
24 uri = self.server_uri
25 super(CouchDatabase, self)._reconnect(uri=uri)
26 # pylint: enable=W0221
27+
28+ def glib_callback_for_changes(self, callback, # pylint: disable=R0914
29+ error_callback=None, since=None):
30+ """Set up a callback to be called, asynchronously, whenever a
31+ change is fired by this database's CouchDB _changes feed.
32+ Note: this callback requires the glib mainloop, but it does
33+ not start it. It is the caller's responsibility to run the glib
34+ mainloop.
35+ The since parameter is a CouchDB sequence number from which you
36+ wish to monitor changes. The default, None, means to monitor changes
37+ from now on. If the caller remembers the last seqno it saw, then
38+ it can pass that as since to get all changes since that last seqno
39+ sent to the callback as well as any future changes.
40+ The callback is called with one argument, a dictionary parsed from
41+ the CouchDB changes feed item, containing id, seq, and changes
42+ members."""
43+ from gi.repository import Soup # pylint: disable=E0611
44+ from oauth import oauth
45+ import cgi, json
46+ session = Soup.SessionAsync()
47+
48+ # inner functions to handle changes when they arrive
49+ def got_chunk(msg, chunkbuffer): # pylint: disable=W0613
50+ """Internal function to handle one line from _changes."""
51+ data = ''.join([chr(x) for x in chunkbuffer.get_data()])
52+ if not data.strip():
53+ # got a heartbeat
54+ return
55+ try:
56+ actual_data = json.loads(data)
57+ except ValueError:
58+ # weird, we got something that wasn't JSON. Ignore it.
59+ if error_callback is not None:
60+ error_callback({
61+ "reason": "Got some data that wasn't JSON",
62+ "error": "could_not_parse_json",
63+ "data": data
64+ })
65+ return
66+ if "id" not in actual_data or "changes" not in actual_data or \
67+ "seq" not in actual_data:
68+ if error_callback is not None:
69+ error_callback(actual_data)
70+ return
71+ callback(actual_data)
72+
73+ def complete(*args): # pylint: disable=W0613
74+ """Internal function called if connection ever completes."""
75+ # should never get called, because connection is long-lived
76+ # it is called when the loop is ended, though.
77+ pass
78+
79+ # make sure session does not quit when function ends
80+ self._soupsessions.append(session)
81+
82+ # Calculate the changes URI for this database, and oauth-sign it
83+ # so that libsoup is able to fetch it
84+ params = dict(feed="continuous",
85+ heartbeat=1000 # newline every 10 seconds to keep connection alive
86+ )
87+ if since is None:
88+ params["since"] = self.db.info()["update_seq"]
89+ else:
90+ params["since"] = since
91+ changes_feed_uri = couchdburi( # pylint: disable=W0142
92+ self._server.resource.uri, self.db.name, "_changes",
93+ **params)
94+ # note: this function does its own oauth signing rather than calling
95+ # the similar oauth signing stuff in server_base.
96+ oauth_data = \
97+ self._server.resource.http.oauth_data # pylint: disable=E1103
98+ consumer = oauth.OAuthConsumer(oauth_data["consumer_key"],
99+ oauth_data["consumer_secret"])
100+ token = oauth.OAuthToken(oauth_data["token"],
101+ oauth_data["token_secret"])
102+ parts = urlparse.urlparse(changes_feed_uri)
103+ query = list(parts)[4]
104+ querystr_as_dict = dict(cgi.parse_qsl(query))
105+ req = oauth.OAuthRequest.from_consumer_and_token(
106+ consumer,
107+ token,
108+ http_method = "GET",
109+ http_url = changes_feed_uri,
110+ parameters = querystr_as_dict
111+ )
112+ req.sign_request(oauth.OAuthSignatureMethod_HMAC_SHA1(), consumer,
113+ token)
114+ signed_changes_feed_uri = req.to_url()
115+
116+ msg = Soup.Message.new("GET", signed_changes_feed_uri)
117+ session.queue_message(msg, complete, signed_changes_feed_uri)
118+ msg.connect("got-chunk", got_chunk)
119+
120
121=== modified file 'desktopcouch/records/tests/test_server.py'
122--- desktopcouch/records/tests/test_server.py 2010-11-16 13:52:34 +0000
123+++ desktopcouch/records/tests/test_server.py 2010-11-16 14:16:13 +0000
124@@ -690,3 +690,42 @@
125 self.fail()
126 except FieldsConflict, e:
127 self.assertEqual({('field1',): (22, 11)}, e.conflicts)
128+
129+ def test_glib_callback_for_changes(self):
130+ """Test glib_callback_for_changes method"""
131+ import gobject
132+
133+ # set var on namespace class as per example 7 at
134+ # http://www.saltycrane.com/blog/2008/01/python-variable-scope-notes/
135+ # because Python 2 can't set outer-but-non-global variables
136+ class PythonCannotSetNonGlobalOuterScopeVars: # pylint: disable=R0903
137+ """Internal class to provide namespace for outer variable."""
138+ def __init__(self):
139+ """Pointless init method just to make pylint happy."""
140+ self.success = False
141+ p = PythonCannotSetNonGlobalOuterScopeVars()
142+
143+ loop = gobject.MainLoop()
144+ def callback(data): # pylint: disable=W0613
145+ """Callback handler to confirm that a callback occurs."""
146+ p.success = True
147+ loop.quit()
148+ def timeout():
149+ """Timeout handler to catch failure to call the callback."""
150+ loop.quit()
151+ def insert_record():
152+ """Trigger a record insertion (and thus a callback)."""
153+ self.database.put_record(Record({
154+ "first": "value1", "second": "value2",
155+ "record_type": "callbacktest.com"}))
156+
157+ # Set up the callback, then in 1s insert a record; if the callback
158+ # works, then that insert will call the callback; if it doesn't,
159+ # then the timeout will be called.
160+
161+ self.database.glib_callback_for_changes(callback)
162+ gobject.timeout_add(1000, insert_record)
163+ gobject.timeout_add(3000, timeout)
164+ loop.run()
165+ self.assertTrue(p.success)
166+

Subscribers

People subscribed via source and target branches