bucket.getFiles should document (at the top) the `on()` way of iterating.

[jgeewax]

Right now, in http://googlecloudplatform.github.io/gcloud-node/#/docs/v0.15.0/storage/bucket?method=getFiles

bucket.getFiles(function(err, files, nextQuery, apiResponse) {
  if (nextQuery) {
    // nextQuery will be non-null if there are more results.
    bucket.getFiles(nextQuery, function(err, files, nextQ, apiResponse) {});
  }
});

which is ... absurdly confusing -- I just want to iterate through all the files.

Can we change the examples to document the way to iterate through all files, and abort the traversal early if necessary?

bucket.getFiles().on('data', function(file) {
  console.log(file);
  if (some condition) {
    this.end();
  }
});

[jgeewax]

/cc @stephenplusplus : Kicking to you. I'm watching someone struggle with this right now.... and the recursive traversal stuff is confusing the hell out of them.

[stephenplusplus]

Separate and name the function to make it easier. We can document how to use streams, but it really isn't the solution. Streams are best when you indend to consume and pass-through all of the data to a connecting stream. It would be best to have a "recurse for me, then give me all of the results in my callback that only gets called once" option and avoid the concept of terminating early. The user always has pageSize and maxResults to avoid getting ridiculous size result sets.

[stephenplusplus]

I'll put a better proposal together tomorrow to demo what I'm thinking.

[jgeewax]

OK -- just the whole recursive calling next query... is confusing. People want to say "call this method again on the next page"...

[stephenplusplus]

just the whole recursive calling next query... is confusing.

Yeah, it is. But it's the most unsurprising implementation. I don't think it would be less confusing to tell the user "your callback is going to be called an unknown amount of times, and each time the result set will be of a different length." I would rather use the callback once after we have all of the results consumed, and not even let the user know the nextQuery/API-determined pagination exists.

As far as the streams go, I added support for the premature ending of a stream as part of the search PR: stephenplusplus@abca6f1 -- after that lands, I'll start to use stream-router throughout the rest of the library.

The only thing about using streams to solve this problem is, it's kind of a weird use of them. The .on('data') API is nicer to deal with, but it's not typically where you see streams being used. A better use case for streams could be something like getting a bunch of data, filtering it down to a set you care about, parsing the metadata out of them and displaying the results in a terminal (bucket.getFiles().pipe(filterFilesOverTenMb).pipe(parseMetadata).pipe(process.stdout)). Streams usually aren't about getting a large set of results, then closing the pipe once you've found the result you're looking for.

Still though, supporting pre-mature closing is probably a safe plan, since the user has ultimate say over how they want to do things.

I still haven't thought of a great API to improve how nextQuery paginating works currently (other than removing it completely), but it's on my mind 😵

[jgeewax]

I don't think it would be less confusing to tell the user "your callback is going to be called an unknown amount of times, and each time the result set will be of a different length."

But it's ... already being called an unknown number of times with the recursive way, right?

---

I would rather use the callback once after we have all of the results consumed, and not event let the user know the nextQuery/API-determined pagination exists.

Hmm... I really appreciated the idea of being able to say "when I've gotten all the items in this chunk, go go again"... this.nextPage() would have solved that problem....

To put it in comparison to Python, we have an Iterator that basically lets you do:

for item in bucket.list_objects():
  print item
  if item.name == 'whatever':
    break

Which is paginating stuff behind the scenes. This is (I think) the .on('data', ...) syntax which... I'm OK with .

[stephenplusplus]

But it's ... already being called an unknown number of times with the recursive way, right?

It depends what we're talking about. Going off of "People want to say "call this method again on the next page"...", I thought you meant you wanted getFiles to recurse itself, and execute that callback with each result set returned from the API.

As far as "nextQuery()" vs "nextQuery" being an object, I fought my case in that other issue. I think it's better to give an object to the user, which I think is where we landed. That, plus documenting how to use the streams to early-exit. There will be a big sweep of all the dual methods soon after Search lands. I have to go through and implement the new stream-router, which will be a great time to catch the docs up.

This is (I think) the .on('data', ...) syntax which... I'm OK with .

Yeah, it will work this way.

[jgeewax]

I thought you meant you wanted getFiles to recurse itself

I think I did mean that, but if it's not acceptable... then being able to say that "we'll run this function for each page until you tell us to stop (or we've gone through all the items)" is fine too...

Bottom line though is... the whole nextQuery thing, and having to re-establish the callback again, or define it elsewhere and pass it in that calls the next query with a reference to itself and all that stuff is... not the way we should be telling users "here's how you page through all the data". It's hugely confusing...

[stephenplusplus]

I'd like to find a higher level solution for the nextQuery process as well. We can change nextQuery the object into a continueProcessing() function. Now I remember (I think) that is where we left off in our last discussion.

[jgeewax]

Or something like nextQuery.runAgain(); or something of that nature...?

[stephenplusplus]

Something like that might work. I'll be thinking on it 💭