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
Fixes #264: Store Posts into Redis #411
Conversation
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 jumping on this, I want to use it tomorrow with what I'm doing in #408.
|
||
exports.workerCallback = async function(job) { | ||
const { url } = job.data; | ||
const posts = await feedparser(url); |
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.
Can you rebase on master
please? We have updated the way this workerCallback
does its thing.
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.
Will do!
src/backend/feed/worker.js
Outdated
}; | ||
|
||
// eslint-disable-next-line prettier/prettier |
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.
Why?
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.
ESLint was complaining about an issue with function () and prettier kept doing changing my code which I had written function() to function ()
feedQueue.process(this.workerCallback); | ||
feedQueue.process(exports.workerCallback); | ||
feedQueue.on('completed', (job, results) => { | ||
if (results.length > 0) { |
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.
Is this necessary? If it's 0, won't it just never call the forEach
callback? I'd remove it.
src/backend/feed/worker.js
Outdated
if (results.length > 0) { | ||
results.forEach(result => { | ||
console.log(`Result: ${result.title}`); | ||
// We have an issure here, addPost is an async function. Should we make the start |
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.
Yes, we also need to update how you do this "loop" when you have promises for each:
await Promise.all(results.map(async result => storate.addPost(result)))
And yeah, we should probably make start
async.
Fixed and merged in class. |
This PR is still a WIP and builds on top of the work done for storage.js.
feedParser
const processedPost
On completion of the processing, it should then store each individual post(result) into Redis by using
storage.addPost()
. There's something we may have to look at though as storage.addPost is an async function. Should we make the exports.start function an async function so we can use await?