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

Get data for [Discord] badges via OVH server proxies #4956

Merged
merged 2 commits into from
Apr 27, 2020

Conversation

calebcartwright
Copy link
Member

@calebcartwright calebcartwright commented Apr 27, 2020

There's more details on the underlying issue in our Discord channels, but for a tl;dr..

Discord throttles calls to their API, and they'd whitelisted our 3 OVH servers previously to allow us to get the requisite data. When we routed img.shields.io traffic over to our Heroku environment we lost that whitelisting and our badge servers (on Heroku) hit the rate limits pretty quickly resulting in invalid for all our Discord badges.

This is a temporary fix to get the Discord badges working by having our Heroku-based badge servers use the OVH servers as a proxy to get the Discord data.

@calebcartwright calebcartwright added the service-badge New or updated service badge label Apr 27, 2020
@shields-cd shields-cd temporarily deployed to shields-staging-pr-4956 April 27, 2020 03:38 Inactive
@shields-ci
Copy link

shields-ci commented Apr 27, 2020

Warnings
⚠️ This PR modified the server but none of its tests.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for discord but not its test code.
That's okay so long as it's refactoring existing code.
Messages
📖 ✨ Thanks for your contribution to Shields, @calebcartwright!

Generated by 🚫 dangerJS against b67d55a

paulmelnikow
paulmelnikow previously approved these changes Apr 27, 2020
@@ -62,6 +67,11 @@ module.exports = class Discord extends BaseJsonService {
}
}

constructor(context, config) {
super(context, config)
this._runningOnHeroku = config.shieldsProductionHerokuHacks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we sync up these names? e.g. use shieldsProductionHerokuHacks for the instance var?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, missed that one somehow. updating now

@paulmelnikow
Copy link
Member

Thanks for addressing this, Caleb!

@AugusDogus AugusDogus mentioned this pull request Apr 27, 2020
3 tasks
@dodocodes
Copy link

👀 thank you for this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge New or updated service badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants