Switch [OpenCollective] badges to use GraphQL and auth #9387
Conversation
* update opencollective to api v2 * fix tests * fix: do not filter by accountType for opencollective/all * remove 404 * remove required in schema * cnt -> count * keep by-tier code as-is --------- Co-authored-by: repo-ranger[bot] <39074581+repo-ranger[bot]@users.noreply.github.com>
chris48s
added
the
service-badge
| @@ -218,7 +218,7 @@ installation access to private npm packages | |||
|
|
|||
| [npm token]: https://docs.npmjs.com/getting-started/working_with_tokens | |||
|
|
|||
| ## Open Build Service | |||
| ### Open Build Service | |||
There was a problem hiding this comment.
This isn't really related to this PR. I just noticed this heading level was wrong while I was editing the file.
|
| class DummyOpencollectiveService extends OpencollectiveBase { | ||
| static route = this.buildRoute('dummy') | ||
|
|
||
| async handle({ collective }) { | ||
| const data = await this.fetchCollectiveInfo({ | ||
| collective, | ||
| accountType: [], | ||
| }) | ||
| return this.constructor.render(this.getCount(data)) | ||
| } | ||
| } |
There was a problem hiding this comment.
No concerns with using a dummy child class, though it does remind of a scenario we ran into with some of the GitLab classes a ways back where we forgot to wire up the auth for one or two of them. As a thought/potential future exercise, I wonder if there's a clean way we could consistently test "does every class that's supposed to use the auth" without having to write all the boilerplate for every single class 🤔
There was a problem hiding this comment.
Yeah it is a fair point, but the pattern here is not not unique to this service or PR. We use the same approach in other places e.g: https://github.com/badges/shields/blob/master/services/stackexchange/stackexchange-base.spec.js
I've raised #9493 as I agree it is worth improving
|
I'm going to hold off merging this just yet as I don't think I've set the OpenCollective token env vars in staging/prod yet. Will sort that out and get a deploy done soon though. |
Closes #9345
Refs #9345 (comment)
Refs #9346 (comment)
This PR takes the code changes from #9346 and makes them viable for production traffic by adding auth and increasing the cache.
I've already reviewed c1a2593 in #9345 so it is only really the 3 additional commits that need a look here.
As things stand, we're currently using a more than 100 reqs/minute at busy times, but also we only cache these badges for 2 mins at the moment. Lets bump that up to 15 mins. That should keep us in line.
Potentially we could exceed this in future if OpenCollective badges become more popular. However I feel like if we need it, OpenCollective is a bit more of a human-scale organisation to ask for an increase than Youtube or DockerHub, for example.
I've made an OpenCollective account attached to the team AT shields.io email that has access to nothing and generated an API token with no scopes that we can use in production
I think for tests, review apps and staging, the anonymous rate limit will be OK
TODO: Before deploying this, add the token to the production env vars and shared credentials store