desktopcouch

Merge lp://qastaging/~sil/desktopcouch/viewparams-497143 into lp://qastaging/desktopcouch

  1. viewparams-497143
  2. Merge into trunk
Proposed by Stuart Langridge
Status: Merged
Approved by: Eric Casteleijn
Approved revision: not available
Merged at revision: not available
Proposed branch: lp://qastaging/~sil/desktopcouch/viewparams-497143
Merge into: lp://qastaging/desktopcouch
Diff against target: 53 lines (+28/-2)
2 files modified
desktopcouch/records/server_base.py (+7/-2)
desktopcouch/records/tests/test_server.py (+21/-0)
To merge this branch: bzr merge lp://qastaging/~sil/desktopcouch/viewparams-497143
Reviewer Review Type Date Requested Status
Eric Casteleijn (community) Approve
Nicola Larosa (community) Approve
Review via email: mp+16219@code.qastaging.launchpad.net

Commit message

Allow execute_view to take a dict of additional parameters, such as { "descending": True }, which can be passed on to the underlying view, thus allowing extra params to be passed onto the CouchDB request.

To post a comment you must log in.
Revision history for this message
Stuart Langridge (sil) wrote :

Allow execute_view to take a dict of additional parameters, such as { "descending": True }, which can be passed on to the underlying view, thus allowing extra params to be passed onto the CouchDB request. See https://answers.edge.launchpad.net/desktopcouch/+question/93862 for a user request for this.

Revision history for this message
Nicola Larosa (teknico) wrote :

Nice code, tests pass.

review: Approve
Revision history for this message
Eric Casteleijn (thisfred) wrote :

Looks fine and tests pass, but any reason not to just have:

def execute_view(self, view_name, design_doc=DEFAULT_DESIGN_DOCUMENT, **params):
    """Execute view and return results."""
...

    return self.db.view(view_id_fmt % locals(), **params)

Also, now that I'm on a new machine, I again notice that running the tests needs access to the keyring, which it absolutely should not.

And I get a weird twisted error at the beginning of the tests:

  File "/usr/lib/python2.6/dist-packages/twisted/python/usage.py", line 241, in parseOptions
    self.postOptions()
  File "/usr/lib/python2.6/dist-packages/twisted/scripts/trial.py", line 293, in postOptions
    self['reporter'] = self._loadReporterByName(self['reporter'])
  File "/usr/lib/python2.6/dist-packages/twisted/scripts/trial.py", line 279, in _loadReporterByName
    for p in plugin.getPlugins(itrial.IReporter):
  File "/usr/lib/python2.6/dist-packages/twisted/plugin.py", line 200, in getPlugins
    allDropins = getCache(package)
--- <exception caught here> ---
  File "/usr/lib/python2.6/dist-packages/twisted/plugin.py", line 179, in getCache
    dropinPath.setContent(pickle.dumps(dropinDotCache))
  File "/usr/lib/python2.6/dist-packages/twisted/python/filepath.py", line 623, in setContent
    f = sib.open('w')
  File "/usr/lib/python2.6/dist-packages/twisted/python/filepath.py", line 433, in open
    return open(self.path, mode+'b')
exceptions.IOError: [Errno 13] Permission denied: '/usr/lib/python2.6/dist-packages/twisted/plugins/dropin.cache.new'

review: Needs Information
Revision history for this message
Stuart Langridge (sil) wrote :

The reason I don't just pass all unknown parameters directly to the underlying view, and instead require a dict of parameters, is in case we decide to add additional things to that function later. If Eric and Chad think it should be changed then I'll change it.

Has this branch introduced a dependency on the keyring? If not, then it'll be better to approve this and then fix the problem in another branch.

Revision history for this message
Eric Casteleijn (thisfred) wrote :

> The reason I don't just pass all unknown parameters directly to the underlying
> view, and instead require a dict of parameters, is in case we decide to add
> additional things to that function later. If Eric and Chad think it should be
> changed then I'll change it.

I'm +0: if you add additional things later, you can always change it then, right? To me it seems that that code just doesn't do anything, so I asked, in case I was misunderstanding something (it has been known to happen ;)

> Has this branch introduced a dependency on the keyring? If not, then it'll be
> better to approve this and then fix the problem in another branch.

Almost certainly not, which is why I didn't mark it needs fixing.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'desktopcouch/records/server_base.py'
2--- desktopcouch/records/server_base.py 2009-12-07 18:50:48 +0000
3+++ desktopcouch/records/server_base.py 2009-12-15 21:02:14 +0000
4@@ -257,13 +257,18 @@
5
6 return deleted_data
7
8- def execute_view(self, view_name, design_doc=DEFAULT_DESIGN_DOCUMENT):
9+ def execute_view(self, view_name, design_doc=DEFAULT_DESIGN_DOCUMENT,
10+ params=None):
11 """Execute view and return results."""
12 if design_doc is None:
13 design_doc = view_name
14
15 view_id_fmt = "_design/%(design_doc)s/_view/%(view_name)s"
16- return self.db.view(view_id_fmt % locals())
17+ if params:
18+ paramdict = params
19+ else:
20+ paramdict = {}
21+ return self.db.view(view_id_fmt % locals(), **paramdict)
22
23 def add_view(self, view_name, map_js, reduce_js,
24 design_doc=DEFAULT_DESIGN_DOCUMENT):
25
26=== modified file 'desktopcouch/records/tests/test_server.py'
27--- desktopcouch/records/tests/test_server.py 2009-12-07 20:10:00 +0000
28+++ desktopcouch/records/tests/test_server.py 2009-12-15 21:02:14 +0000
29@@ -349,3 +349,24 @@
30
31 # We can remove records with attachments.
32 self.database.delete_record(record_id)
33+
34+ def test_view_fetch(self):
35+ design_doc = "test_view_fetch"
36+ view1_name = "unit_tests_are_great_yeah"
37+
38+ map_js = """function(doc) { emit(doc._id, null) }"""
39+
40+ self.database.add_view(view1_name, map_js, None, design_doc)
41+ data = [i.key for i in
42+ list(self.database.execute_view(view1_name, design_doc))]
43+ # ordinary requests are in key order
44+ self.assertEqual(data, sorted(data))
45+
46+ # now request descending order and confirm that it *is* descending
47+ descdata = [i.key for i in
48+ list(self.database.execute_view(view1_name, design_doc,
49+ {"descending": True}))]
50+ self.assertEqual(descdata, list(reversed(sorted(data))))
51+
52+ self.database.delete_view(view1_name, design_doc)
53+

Subscribers

People subscribed via source and target branches