-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Refactor move inbound mail parser to worker #4986
Conversation
Just a note, it will require a terraform change aswell* cc @Cliftonz |
…ser-to-worker # Conflicts: # apps/worker/src/app/workflow/workflow.module.ts
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.
from the code perspective it looks good to me! 🔥
@Cliftonz we would need to implement appropriate cloud infra changes after this PR is merged
@djabarovgeorge I see the spell check failing for the moment. however I want to confirm that now the workers will be processing the inbound mail and it will go like: email -> dns -> inbound-email service-> queue -> email parse (on worker) -> standard queue If this is true then we can merge this in and the general worker service should be able to handle this. |
@Cliftonz It will be even shorter |
@djabarovgeorge Then I do not think we need to make any changes as the general worker service can handle the load of at most 10 jobs per month. |
@djabarovgeorge There are a few conflicts but I would like to get this merged in soon |
…ser-to-worker # Conflicts: # apps/worker/src/app/workflow/workflow.module.ts
❌ Deploy Preview for dev-web-novu failed.
|
Tend to agree here 💪 |
What change does this PR introduce?
Move the inbound mail parser worker to the worker app.
Why was this change needed?
There is no reason for us to spin the inbound mail parser worker in the API app.
Other information (Screenshots)