GROW-118 Dynamic loan urls and images from Ml recommender #1881
Conversation
# Conflicts: # package.json
server/live-loan-router.js
Outdated
| try { | ||
| const loanData = await getRecommendedLoans('img', userId/* , offset */); | ||
| const loanCardImgBuffer = await drawLoanCard(loanData[0]); // TODO: offset? | ||
| // TODO: Add loanCard img to memcache? |
There was a problem hiding this comment.
Add loanCard img to memcache here?
There was a problem hiding this comment.
We need to come up with a system to reduce the number of images that might be cached. As I mentioned before, rounding the currently funded about to the nearest 20th percent would be a good start. Then you could have cache keys for images like so... live-loan-##loanid##-0, live-loan-##loanid##-20, live-loan-##loanid##-40 etc...
This would at least give us a baseline of measurement to ensure we have caching instances that can hold enough entries.
There was a problem hiding this comment.
Then your process will be more like...
const loanCardCacheKey = live-loan-##loanid##-0 let cachedLoanCardImage = retrieveFromCache(live-loan-##loanid##-0) if (!cachedLoanCardImage) { cachedLoanCardImage = await drawLoanCard() } ... res.send(cachedLoanCardImage);
There was a problem hiding this comment.
We could also just cache one image for a loan, and use a ttl rather than a cache key based on the loan progress. (The cached value could be json, allowing you to store and check the time it was cached)
There was a problem hiding this comment.
@emuvente I'd be fine with that if we can build in a "delay" of sorts so that we're not re-generating the image for small changes in the funded amount. my interest is reduce re-generating the image repeatedly (aka load on the express object) and to prevent excessive cache entries for each loan...
There was a problem hiding this comment.
The ttl would handle that delay, as the image would not be regenerated while the cached image is young enough. It would also handle the excessive entries for each loan as only 1 image would ever be cached for a loan.
| return path.join(__dirname, '../../../src/assets/fonts', name); | ||
| } | ||
| registerFont(fontFile('PostGrotesk-Medium.ttf'), { family: 'Kiva Post Grot', weight: '400' }); | ||
| registerFont(fontFile('PostGrotesk-MediumItalic.ttf'), { family: 'Kiva Post Grot', weight: '400', style: 'italic' }); |
There was a problem hiding this comment.
TODO: Having issues with node-canvas and registering fonts. So far only the first font is able to be used. Still working on this but I don't think it'll be a dealbreaker.
There was a problem hiding this comment.
What if you used a different font family name for each one?
There was a problem hiding this comment.
No luck there, I think I may need to patch the fonts or wait for a new release of node-canvas - Automattic/node-canvas#1572
|
To ensure that the data is accurate, I would recommend against using different graphql queries for the url and img handling. There should be a single get recommended loans call that is cached (old dynamic loan code would cache this data for 30 minutes, but I could see 10 minutes or less being ok too) and used by both the url and img handlers, as that prevents the situation of the url query getting a different loan than the image query. |
With regard to caching the loan query...at first I was thinking we wouldn't want to...and that may still be the case... This is a user level query so what's the likely hood of the cache result ever getting used? |
It's going to be used at least once, because the query is called once for the image, and then a second time for the url when the user clicks on the image and needs to be redirected. |
|
Going to merge this and work on cache improvements next week as hotfixes. |
Couple of things to iron out still, but ready for some review.
To demo:
https://dev-vm-01.kiva.org/live-loan/u/234/img/1
https://dev-vm-01.kiva.org/live-loan/u/234/url/1
A few comments on the files below