wip: refactor abstract mapreduce#9177
Conversation
| throw new Error(row.reason); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
doesn't this make this code path synchronous instead of asynchronous? I have not looked at the bigger picture yet, but my gut feeling says that handling attachments (large binary values) probably wants to be async?
There was a problem hiding this comment.
Good question! I was thinking that its safe on this call since the function addHttpParam is already async and before calling postprocessAttachments we still await the db.fetch and response.json.
I thought that since postprocessAttachments and readAttachmentsAsBlobOrBuffer themselves are synchronous, wrapping the result in a Promise and calling postprocessAttachments on .then() only slightly delays the call? - if not this is definitely where my confusion lies! :)
There was a problem hiding this comment.
Emily and I paired on this and I am now convinced that postprocessAttachments() and further down are all synchronous functions, so that is all fine.
The only thing left here to find out is why the original code does the wrapping in another promise to move the post processing to the next tick.
Maybe @AlbaHerrerias you remember when you made that change [checks notes] a long time ago? ;D
There was a problem hiding this comment.
Hi 👋 unfortunately I do not recall, this was 3 years ago after all. I went to the PR that introduced the change but still no idea, it is true that looks odd
3 participants
did some ES6 refactoring:
converted functions
updatePurgeSeq, saveKeyValues, createTask, getRecentPurgestoasync/awaitto replace promise chainsrefactored postProcessAttatchments
to accept the resolved result (res) directly, wich enables to remove the.then()` on the callremoved unneccessary promise wrappers around calls of:
postProcessAttatchments(is now invoked directly with the resolved result - instead of with.then()chaining)queryPromised(the async function already returns a promise)replaced the use of
fin(final promise factory function) withtry/finallyblockremoved the last
varnote:
currently repeadedly writing out the
defaultstofunctionality, will take care of it next :)