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

Implement got pagination support #3163

Closed
rarkins opened this issue Feb 4, 2019 · 5 comments
Closed

Implement got pagination support #3163

rarkins opened this issue Feb 4, 2019 · 5 comments
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:feature Feature (new functionality)

Comments

@rarkins
Copy link
Collaborator

rarkins commented Feb 4, 2019

What would you like Renovate to be able to do?

Support GitHub's pagination in a got wrapper.

Describe the solution you'd like

I would like to add a lib/util/got/github-pagination.js instance that expands on the default instance and returns a paginated res.body if opts.paginate is set to true in the request.

Describe alternatives you've considered

Writing pagination logic manually at the application layer every time.

Additional context

I started writing lib/util/got/* wrappers already.

@rarkins rarkins added type:feature Feature (new functionality) ready priority-2-high Bugs impacting wide number of users or very important features labels Feb 4, 2019
@rarkins
Copy link
Collaborator Author

rarkins commented Feb 4, 2019

@szmarczak do you have any suggestions for how to do this elegantly? I think I could probably do this with got.create() and handler BUT the default got instance I wrote that uses caching already has a handler so I don't think it's possible to "merge" two instances with handlers, right?

In that case I think I need to create a new got instance that then "calls" my existing one instead of merges with it.

@rarkins
Copy link
Collaborator Author

rarkins commented Feb 4, 2019

Ref: sindresorhus/gh-got#22

@rarkins rarkins added priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others and removed priority-2-high Bugs impacting wide number of users or very important features labels Feb 4, 2019
@szmarczak
Copy link

I don't think it's possible to "merge" two instances with handlers, right?

Of course you can! :)

You're right, handlers should do the job. But pagination should be supported in Got natively.

@rarkins
Copy link
Collaborator Author

rarkins commented Feb 4, 2019

@szmarczak have you ever thought about the semantics for a pagination "API"?

My approach:

  • If the "per page" is specifiable then the app should do it (e.g. set ?per_page=100)
  • App must opt in to pagination (e.g. opts.paginate = true) rather than automatic
  • Paginate all pages by default? Use a second variable for paginationLimit or overload opts.paginate to let it be a number of pages?

@szmarczak
Copy link

have you ever thought about the semantics for a pagination "API"?

Not yet. Let's continue the talk at sindresorhus/got#722

I'll sketch something out.

This was referenced May 30, 2019
@rarkins rarkins added the status:requirements Full requirements are not yet known, so implementation should not be started label Jan 12, 2021
@rarkins rarkins closed this as completed Aug 8, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:requirements Full requirements are not yet known, so implementation should not be started type:feature Feature (new functionality)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants