-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Update [amo] to use v4 API, set custom cacheLength
s
#7586
Conversation
@@ -21,7 +21,7 @@ class BaseAmoService extends BaseJsonService { | |||
async fetch({ addonId }) { | |||
return this._requestJson({ | |||
schema, | |||
url: `https://addons.mozilla.org/api/v3/addons/addon/${addonId}/`, | |||
url: `https://addons.mozilla.org/api/v4/addons/addon/${addonId}/`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were no breaking changes from v3 to v4 affecting any of the values we use on any of these badges
@@ -14,6 +14,8 @@ export default class AmoUsers extends BaseAmoService { | |||
}, | |||
] | |||
|
|||
static _cacheLength = 21600 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@calebcartwright based on your comment in #7584 (comment) I've set downloads/users to 21600
seconds (6 hours). Feel free to tweak it if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No concerns, if anything I'd view this as a reasonable floor. I'd be open to cranking it up a bit too (especially if we need to investigate more load reduction avenues), but think that a full day would have been too much given there'd be windows of proximity to the upstream daily update where the badge could be noticeably outdated for nearly 24 hours
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for picking this up, looks good to me. 👍🏻
Closes #7584