Use v4 API for paginating releases in `check`

concourse / github-release-resource

a resource for github releases

Apache License 2.0

57 stars 58 forks source link

Use v4 API for paginating releases in `check` #102

Closed kirillbilchenko closed 3 years ago

kirillbilchenko commented 3 years ago

Update dependencies fix tests and added graphql query for fetching list of releases

Release Note

When an access_token is provided, check will use the v4 API for paginating releases. This results in much faster/more stable checks on repositories that have many release assets

kirillbilchenko commented 3 years ago

Hey @kirillbilchenko, first question, is there a reason we still have the vendor folder, this might be related to the previous pr but it seems that we can git rid of the vendor folder altogether and the code will still work normally.

Yes, you are right we can remove it, let me do this, I only did update of dependencies and moved to go mod, but have not consider the cleanup.

kirillbilchenko commented 3 years ago

@YoussB thanks for review, more details about graphQL and usage I think you can find in this pr, it was a case for big repos when usual api returning huge payloads https://github.com/concourse/github-release-resource/pull/96. This is a first case, another case if you are using token you have two separate limits for graph ql queries and for api v3 and they are calculated separately, so it will definitely will be win for checks where you have lot's of repos. In this pr I decided to go with the same contracts in return for list of releases. to not rewrite all dependant use cases. But I think this can be extended in future and maybe improved.

YoussB commented 3 years ago

@kirillbilchenko, thanks for the clarification. I didn't think about the payload. Can we at least wrap the graphql calls in a separate helper?

kirillbilchenko commented 3 years ago

@kirillbilchenko, thanks for the clarification. I didn't think about the payload. Can we at least wrap the graphql calls in a separate helper?

Not sure if this make real sense, cuz now all calls are in one class and file and it's easier to understand what's happening there. But again we can wrap but I don't see any real benefit from this for now.

kirillbilchenko commented 3 years ago

@YoussB I moved this list resources to separate file called github_graphql

kirillbilchenko commented 3 years ago

@aoldershaw I updated the pr for cases without token I keep v3 usage