Merge lp:~james-w/lazr.restfulclient/improve-collection-fetch-optimisation into lp:lazr.restfulclient

[James Westby]

[James Westby (james-w)]

Hi,

This change helps with fetching large collections. There is
on optimisation that sets the desired page size if it looks
like a full page will be unneeded, but the way it was coded
meant that the first fetch was always full.

The change here means that the optimisation is more widely applied,
but critically for me you can actually control the size of the fetches
my looping the slices. Some API calls are liable to timeout, so
chunking them smaller allows you to use them, and that's currently
not possible. With this change you can iterate slices to control
the page size.

Thanks,

James

[James Westby (james-w)]

Actually this doesn't allow you to limit the page size
all the time. Once you get beyond the first page it
makes a request with no size specified.

In order to do this properly we would either need a
max-size request variable, would that be feasible for
LP?

Thanks,

James

[Graham Binns (gmb)]

Hi James,

Thanks for this branch. A couple of things:

 1. I don't see any tests for this change. Are there tests that already cover this functionality? If so, how will this change affect them? If not, can you add some?
 2. Formatting multi-line conditionals is always a bit weird. We usually wrap after the first 'and' or 'or'. I've written a patch that fixes this for this branch: http://pastebin.ubuntu.com/558064/

> Actually this doesn't allow you to limit the page size
> all the time. Once you get beyond the first page it
> makes a request with no size specified.
>
> In order to do this properly we would either need a
> max-size request variable, would that be feasible for
> LP?
>
 3. I don't really understand what you mean here (I'm still jetlagged, which doesn't help). Can you explain it further (possibly using words of no more than one syllable)?

review: Needs Information (code)

Unmerged revisions

87. By James Westby on 2010-01-05

Account for the case where there isn't a first page properly.

86. By James Westby on 2010-01-05

Make the optimisation in Collection._get_slice apply to more cases.

Collection._get_slice had an optimisation to fetch smaller pages if it
thought that a full page was unneeded. However, it only applied this on
the second and subsequent fetch. This change makes it apply to the first
fetch as well, as it applies there equally.

=== modified file 'src/lazr/restfulclient/resource.py'
--- src/lazr/restfulclient/resource.py	2009-12-17 15:03:38 +0000
+++ src/lazr/restfulclient/resource.py	2010-01-05 11:41:18 +0000
@@ -753,6 +753,17 @@
 
         # Iterate over pages until we have the correct number of entries.
         while more_needed > 0 and page_url is not None:
+            if (first_page_size is not None
+                    and more_needed > 0 and more_needed < first_page_size):
+                # An optimization: it's likely that we need less than
+                # a full page of entries, because the number we need
+                # is less than the size of the first page we got.
+                # Instead of requesting a full-sized page, we'll
+                # request only the number of entries we think we'll
+                # need. If we're wrong, there's no problem; we'll just
+                # keep looping.
+                page_url = self._with_url_query_variable_set(
+                    page_url, 'ws.size', more_needed)
             representation = simplejson.loads(
                 unicode(self._root._browser.get(page_url)))
             current_page_entries = representation['entries']
@@ -766,16 +777,7 @@
                 break
             if first_page_size is None:
                 first_page_size = len(current_page_entries)
-            if more_needed > 0 and more_needed < first_page_size:
-                # An optimization: it's likely that we need less than
-                # a full page of entries, because the number we need
-                # is less than the size of the first page we got.
-                # Instead of requesting a full-sized page, we'll
-                # request only the number of entries we think we'll
-                # need. If we're wrong, there's no problem; we'll just
-                # keep looping.
-                page_url = self._with_url_query_variable_set(
-                    page_url, 'ws.size', more_needed)
+
 
         if slice.step is not None:
             entry_dicts = entry_dicts[::slice.step]