Add cache to function response #22
Conversation
I tried adding a cache (1h, but can be changed) to help with the commit count problem. It reduced Anshuman Verma's request from 4.5s avg to 185s avg in subsequent requests. The first request took a bit longer. This approach has a problem, though. The stats will only refresh once the cache max-age has expired, I guess it's not that big of a deal, shield.io and similar tend to have cache in place.
|
This one is legit! Maybe consider using |
api/index.js
Outdated
| @@ -189,6 +189,7 @@ module.exports = async (req, res) => { | |||
|
|
|||
| const stats = await fetchStats(username); | |||
|
|
|||
| res.setHeader('Cache-Control', 's-maxage=3600'); | |||
There was a problem hiding this comment.
I think 5min cache would be good enough since we need the frequent updates too
res.setHeader("Cache-Control", "public, max-age=300");
There was a problem hiding this comment.
5 minutes would be good I guess, I made it one hour just to test it.
There was a problem hiding this comment.
yup, good catch, I can move it into a constant or something.
const FIVE_MINUTES = 300;
res.setHeader('Cache-Control', `public, max-age=${FIVE_MINUTES}`);There was a problem hiding this comment.
Create a constants file under src, add all the constants there -> export -> tada
For the const, instead of FIVE_MINUTES we can name it like MAX_AGE_LIMIT_IN_MS
There was a problem hiding this comment.
yup, I commented that to Anurag, but he said he wanted to add that to env so it can easily be changed.
|
I lowered it to 5 minutes. Is there any other place it should be added? I guess it's only needed because of the commit count, but might be worth adding to the pin request. What you think? |
@fransallen What is the difference between the 2? I have not read much about it. I can change it to |
|
@nombrekeff I think you can also add cache headers to pin.js file. Also let's use public, max-age since I checked it works. I did not checked the other one |
|
Sure, I've been reading about it, and |
|
Changed and added it to |
|
@nombrekeff LGTM! thanks for the PR ❤️ |
|
Opps looks like the PR got merged with the commit-counts branch, no issues, i think its good i can work on that branch and merge it directly. |
|
Great! Hope it helps 😄 |
I tried adding a cache (1h, but can be changed) to help with the commit count problem. It reduced Anshuman Verma's request from 4.5s avg to 185s avg in subsequent requests. The first request took a bit longer.
This approach has a problem, though. The stats will only refresh once the cache max-age has expired, I guess it's not that big of a deal, shield.io and similar tend to have cache in place.