Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Support serial eachPage() operations #201

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

evansolomon commented Dec 11, 2013

The eachPage() request helper is very cool, but I'm not sure I always want it to go as fast as possible. If there are many pages, then it could trigger lots of potentially expensive callbacks all at once. I think it would be cool if it acted a bit like a stream that waited for the data to be consumed.

This is a work in progress, so I'm curious to hear what you think. I realize that the same thing could be done by writing my own wrappers around paging, but since eachPage is already there I'd like to use it if I can. I'd also like to do the same with eachItem, but I haven't started working on it yet because pausing the consumption is a little bit trickier since there's no natural point to stop for i/o between items and I didn't want to start pulling in outside dependencies for queue or async management before seeing if you're interested in the idea at all.

The main idea is to be able to do something like this.

s3.listObjects(params).eachPage(function (err, data, done) {
  doSomethingAsyncAndOrExpensive(data, function () {
    // The next page of results isn't fetched until now
    done()
  });
});
Contributor

lsegal commented Dec 13, 2013

At a high level I actually think this might be a good idea. That said, I think there is value for "going as fast as possible". Perhaps having both eachPage and eachPageAsync calls would be useful.

Contributor

evansolomon commented Dec 13, 2013

I thought about doing that.

Just to clarify one thing, the default behavior of eachPage in my branch is still to keep iterating on its own. It only switches into this new mode if the callback you pass accepts 3 arguments (the 3rd is the new callback supplied by eachPage that controls iteration). Not to say that's definitely the way it should work, just that this is how I first went about it.

Contributor

lsegal commented Dec 13, 2013

Ah, good point. Then yes, if it checks the function arity, that's a very good way to go about it in a backward compatible way, then we wouldn't need the *Async option.

Contributor

evansolomon commented Dec 13, 2013

Since it sounds like you're into it, I'll work on doing the same for eachItem.

Contributor

lsegal commented Jul 1, 2014

I finally had a chance to jump back on this. Just merged it via ef4e3e1. This should be in the next release.

Thanks for the contribution! Let me know if you ever get this working for eachItem. Pulling that in should be quicker.

@lsegal lsegal closed this Jul 1, 2014

Contributor

evansolomon commented Jul 1, 2014

Awesome, thank you for picking this up after I orphaned it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment