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
=== modified file 'desktopcouch/records/server_base.py'
--- desktopcouch/records/server_base.py 2009-12-07 18:50:48 +0000
+++ desktopcouch/records/server_base.py 2009-12-15 21:02:14 +0000
@@ -257,13 +257,18 @@
257257
258 return deleted_data258 return deleted_data
259259
260 def execute_view(self, view_name, design_doc=DEFAULT_DESIGN_DOCUMENT):260 def execute_view(self, view_name, design_doc=DEFAULT_DESIGN_DOCUMENT,
261 params=None):
261 """Execute view and return results."""262 """Execute view and return results."""
262 if design_doc is None:263 if design_doc is None:
263 design_doc = view_name264 design_doc = view_name
264265
265 view_id_fmt = "_design/%(design_doc)s/_view/%(view_name)s"266 view_id_fmt = "_design/%(design_doc)s/_view/%(view_name)s"
266 return self.db.view(view_id_fmt % locals())267 if params:
268 paramdict = params
269 else:
270 paramdict = {}
271 return self.db.view(view_id_fmt % locals(), **paramdict)
267272
268 def add_view(self, view_name, map_js, reduce_js,273 def add_view(self, view_name, map_js, reduce_js,
269 design_doc=DEFAULT_DESIGN_DOCUMENT):274 design_doc=DEFAULT_DESIGN_DOCUMENT):
270275
=== modified file 'desktopcouch/records/tests/test_server.py'
--- desktopcouch/records/tests/test_server.py 2009-12-07 20:10:00 +0000
+++ desktopcouch/records/tests/test_server.py 2009-12-15 21:02:14 +0000
@@ -349,3 +349,24 @@
349349
350 # We can remove records with attachments.350 # We can remove records with attachments.
351 self.database.delete_record(record_id)351 self.database.delete_record(record_id)
352
353 def test_view_fetch(self):
354 design_doc = "test_view_fetch"
355 view1_name = "unit_tests_are_great_yeah"
356
357 map_js = """function(doc) { emit(doc._id, null) }"""
358
359 self.database.add_view(view1_name, map_js, None, design_doc)
360 data = [i.key for i in
361 list(self.database.execute_view(view1_name, design_doc))]
362 # ordinary requests are in key order
363 self.assertEqual(data, sorted(data))
364
365 # now request descending order and confirm that it *is* descending
366 descdata = [i.key for i in
367 list(self.database.execute_view(view1_name, design_doc,
368 {"descending": True}))]
369 self.assertEqual(descdata, list(reversed(sorted(data))))
370
371 self.database.delete_view(view1_name, design_doc)
372

Subscribers

People subscribed via source and target branches