Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some solution is needed for controlled paging #43

Closed
vermiculus opened this issue Jan 24, 2018 · 9 comments
Closed

Some solution is needed for controlled paging #43

vermiculus opened this issue Jan 24, 2018 · 9 comments

Comments

@vermiculus
Copy link
Contributor

The unpaginate argument is the idealized behavior of any API request, but it's often inappropriate when working with very large repositories with many hundreds (perhaps even thousands) of objects. In these cases, it's a better user experience to actually page the results.

Related vermiculus/magithub#152.

@vermiculus
Copy link
Contributor Author

vermiculus commented Jan 24, 2018

I can try to work on a solution some time next week, but honestly I can't think of anything satisfactory without introducing either

  • a dynamic variable set in ghub-request that indicates if a link exists (regardless of unpaginate)
  • some abstracted response to store the link to the next page of responses along with functions ghub-next-page (or somesuch) to interact with that abstraction. This would be a breaking change, but it's probably more powerful while maintaining some simplicity.

What are your thoughts?

@tarsius
Copy link
Member

tarsius commented Jan 24, 2018

Sounds like you want to handle this outside of ghub.

How about allowing ghub-request's UNPAGINATE argument to be an integer or even (NUMBER-OF-REQUESTS . ITEMS-PER-REQUEST) instead?

@vermiculus
Copy link
Contributor Author

I'm not against ghub handling it, but it does strike me as counter to ghub's minimalist goals. Then again, how do you plan to handle this problem in magit? It's worth some thought.

That consideration notwithstanding, there's only one thing needed for controlled pagination -- three for complete control: where to start returning results. IIRC, this is usually controlled by the page query parameter. The other two are:

  • how many objects per page (default determined by API; I think it's 30)
  • how many pages to retrieve (default 1)

For the first piece -- the page piece -- ghub clients can't give meaningful input unless they know where the previous results dropped off. There has to be some returned value -- whether that's dynamic state (which is gross, I agree) or by a formally returned value (which will be a breaking change). While they may exist, I'm not aware of other options ☹️

@vermiculus
Copy link
Contributor Author

vermiculus commented Jan 24, 2018

I missed an option -- it is possible (though a duplication of work across clients) to keep track of which page of each resource was last retrieved. That should be possible to do at the ghub+ level, at which point I'd probably opt for cl-structs (blech!).

Unless there's a better way to do it, of course.

Magit will eventually have this problem, too, since many repositories maintain >30 open PRs:

@tarsius
Copy link
Member

tarsius commented Jan 24, 2018

Does the graphql api also return paginated results?

@vermiculus
Copy link
Contributor Author

It does, but it's a little different. GraphQL has a concept of a cursor that it prefers over other methods.

Wondering what you're thinking, here -- as far as ghub is concerned, v4 GraphQL is accessed the same way as v3 REST (I'm pretty sure); see examples.el@b0bd6c.

@vermiculus
Copy link
Contributor Author

While setting a dynamic variable would be an easy fix, I don't think it's the right thing to do -- especially with the impending advent of multithreaded elisp.

@tarsius tarsius mentioned this issue Feb 16, 2018
@tarsius tarsius mentioned this issue Mar 8, 2018
15 tasks
@tarsius
Copy link
Member

tarsius commented Mar 12, 2018

I have just pushed better unpaginating support and later today I will probably push support for asynchronous requests, which will improve unpaginating further.

(length (ghub-get "/users/tarsius/repos"))

(length (ghub-get "/users/tarsius/repos" '((per_page . 100))))

(length (ghub-get "/users/tarsius/repos" nil :unpaginate t))

(length (ghub-get "/users/tarsius/repos" '((per_page . 4)) :unpaginate t))

(length (ghub-get "/users/tarsius/repos" '((per_page . 4)) :unpaginate 3))

Asynchronous requests will allow controlled unpaginating without the need for global state.

(ghub-get "/users/tarsius/repos" nil
          :unpaginate t
          :callback (lambda (value _headers _status _args)
                      (setq my-value value)))

(ghub-get "/users/tarsius/repos" nil
          :callback (lambda (value _headers _status args)
		      (unless (ghub-continue args)
			(setq my-value value))))

These two forms are basically equivalent, but the second could be extended to only unpaginate if some condition is met (most likely if not too much time has passed already).

@tarsius tarsius closed this as completed Mar 12, 2018
@vermiculus
Copy link
Contributor Author

thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants