octokat.js icon indicating copy to clipboard operation
octokat.js copied to clipboard

Unable to "fetch" all with paginated results.

Open matthewbauer opened this issue 10 years ago • 9 comments
trafficstars

Could we get some way to get all results in the paginated results?

matthewbauer avatar Jun 29 '15 18:06 matthewbauer

I'm using the following helper code in another project: https://github.com/philschatz/gh-board/blob/master/src/helpers.js#L1

// The following is ES6 but should just require changing the anonymous functions syntax
function fetchAll(fn, args) {
  let acc = []; // Accumulated results
  let p = new Promise((resolve, reject) => {
    fn(args).then((val) => {
      acc = acc.concat(val);
      if (val.nextPage) {
        return fetchAll(val.nextPage).then((val2) => {
          acc = acc.concat(val2);
          resolve(acc);
        }, reject);
      } else {
        resolve(acc);
      }
    }, reject);
  });
  return p;
}

fetchAll(octo.repos('philschatz', 'octokat.js').issues.fetch).then((allIssues) => {
  console.log(allIssues.length);
});

It's not the prettiest solution but is that enough of a workaround for now?

philschatz avatar Jun 29 '15 19:06 philschatz

+1

AndersDJohnson avatar Sep 27 '15 22:09 AndersDJohnson

I've used something similar in one of my projects:

  var getAll = function (fn, lastIdRead, resultSet, callback) {
    fn({since: lastIdRead}, function (err, result) {
      if (err) {
        callback(err)
        return
      }

      if (!result || result.length == 0) {
        callback(null, resultSet)
      } else {
        var lastId = result[result.length - 1].id
        getAll(fn, lastId, resultSet.concat(result), callback)
      }
    })
  }

bitoiu avatar Sep 28 '15 01:09 bitoiu

I've used something like this, relying on caolan/async. Includes caching in localStorage, for use in browser.

function getData(cacheKey, octoInstance, octoHandler, done) {

  var cached = localStorage.getItem(cacheKey);

  if (cached) {
    done(null, JSON.parse(cached));
    return;
  }

  var promiser = function () { return octoHandler(octoInstance); };
  var allData = [];

  async.doWhilst(
    function (cb) {
      promiser().then(
        function (data) {
          allData = allData.concat(data);
          promiser = data.nextPage;
          cb();
        },
        function (err) {
          cb(err);
        }
      );
    },
    function () {
      return promiser;
    },
    function (err) {
      if (err) throw err;

      localStorage.setItem(cacheKey, JSON.stringify(allData));

      done(err, allData);
    }
  );
};

where octoHandler is a function that takes an Octokat instance and returns a promise for a call on that instance, e.g.:

function octoHandler(octoInstance) {
  return octoInstance.repos('philschatz', 'octokat.js').issues.fetch;
}
var octoInstance = new Octokat();
getData('issues:philschatz/octokat.js', octoInstance, octoHandler, function (err, allIssues) {
  // do something with all issues, unless error.
});

AndersDJohnson avatar Sep 29 '15 03:09 AndersDJohnson

@adjohnson916 not sure about a manual cache key. It seems like the second time I do the request I'll most likely get old results or did I miss understand your code?

bitoiu avatar Sep 29 '15 04:09 bitoiu

For what it's worth, you can do caching to localStorage "relatively" easily.

The Octokat constructor takes a cacheHandler field (undocumented) that needs to implement get(method, path) and add(method, path, eTag, data, status).

Caching paged results is a little annoying (see #67 for a fix) because the next/previous link headers are not part of the array that is cached.

To get it to work with the current Octokat, here's what I do: philschatz/gh-board/github-client.js#L7-L66

Saving to localStorage is slow so the code linked above:

  1. waits pendingTimeout before saving instead of saving on every request
  2. blows the cache when > 200 URLs are in the cache

And to save the link headers on the Array when saving/retrieving to localStorage I save them as separate fields and then place them back on the array.

You can go to http://philschatz.com/gh-board/ , choose a repo and then run JSON.parse(sessionStorage['octokat-cache']) in the console to see the cache. Also, clicking "Refresh" will not decrease your API rate limit.

Let me know if you find the cacheHandler param to be useful and I can add it to the README

philschatz avatar Sep 29 '15 04:09 philschatz

@bitoiu Yes, my example was mostly meant to show how I'm doing pagination with caolan/async. I've just updated it to be simpler with doWhilst instead of whilst.

I'm not pushing for adoption of my perhaps naïve localStorage implementation. Yes, it caches permanently and relies on manual cache key updates. That has been useful for me during development to minimize GitHub API rate limiting.

@philschatz Thanks for the info on cacheHandler and how you're using it in gh-board. I might re-work my code to rely on that. It'd be good if that were in the README.

FYI, I'm using these ideas in my work-in-progress adjohnson916/github-issue-rank.

AndersDJohnson avatar Sep 29 '15 09:09 AndersDJohnson

Thanks for the feedback! Sorry it took so long to respond. #81 splits octokat into smaller pieces so users can opt-in to different features (like fetchAll, camelCase, Hypermedia, sending binary files) to address this and other issues people have found.

I hope that helps address this annoying lack-of-a-feature :smile: . Any thoughts/feedback on #81 are definitely appreciated!

philschatz avatar Jan 05 '16 04:01 philschatz

Great - that refactoring will be a nice improvement.

dogweather avatar Jan 05 '16 04:01 dogweather