-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
EmailPlugin: Handle async Email data loading in worker instead of main process #2045
Comments
For future reference @is0utfitters |
Martijn, thanks for these issues - I think they all make sense and it would be great to upgrade the core email plugin with this functionality. The only thing we need to be mindful of here is that if we do loadData on the worker, then we won't be able to directly work with the event we are subscribing to, since that data would have to have been serialized to be stored in the job queue in the mean time. So I think we'd probably need an additional mechanism for processing on the worker, as there still will be cases where a lightweight loadData call still makes sense on the server. I'm happy to have you contribute to this - I'll leave it to you to explore the possibilities, and give you feedback on any ideas you come up with. |
Thanks for the response, can you assign this to me so I can find it in my list? I will first investigate the possibilities on this. I was indeed already worrying about serializability of events. But, on the other hand. Anything thats in an event like that should also be fetchable from the DB in the worker right? I do agree that the essential information should be serialized in the Job, so you don't need to refetch stuff from the DB for basic email handling. |
@martijnvdbrug any update on this? |
@dlhck I think we can close this. It's currently not possible because of the un-serializable nature of events from the event bus. A workaround is to create your own plugin for sending emails. It's also not really an issue unless you are using fully serverless, but there are more things in Vendure not serverless proof. And to be honest, not hybrid serverless is working fine for us right now :-) (Google Cloud Run + CPU always on) TL;DR: This issue requires significant changes to the core, and it is also not needed for us anymore. |
Is your feature request related to a problem? Please describe.
A developer (me, this is how I do it), could do resource intensive tasks in the
loadData()
function of an Email handler:Currently this is being done in the main process. It would be smarter, if we do this in the worker instance. Also, for serverless envs, there is a chance your process gets killed, because this data loading happens outside the request context.
Describe the solution you'd like
I would like the EmailHandlers to listen for their events (
OrderPlacedEvent
in the example above) and then push a job to the worker. The worker job would then do the data loading. The email job would now not only be responsible for email sending, but also the email data loading.Describe alternatives you've considered
Just use the email plugin as is.
Additional context
If this is something we want in Vendure core, I would like to contribute, together with #2043 and #2044
(Can we bundle issues somehow?)
The text was updated successfully, but these errors were encountered: