fix: prevent emails from being queued multiple times #19386
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #19386 +/- ##
========================================
Coverage 65.06% 65.06%
========================================
Files 755 755
Lines 71866 71878 +12
Branches 6117 6117
========================================
+ Hits 46759 46769 +10
- Misses 21617 21619 +2
Partials 3490 3490
Flags with carried forward coverage won't be shown. Click here to find out more. |
- Combined email queue sending to single call instead of different branches for testing. - If failing to get jobs -> attempt sending anyway. Failure could be form one bad job which shouldn't completely stop everything.
|
I've modified few things:
|
86733a9 to
d91d28e
Compare
ankush
approved these changes
Dec 22, 2022
mergify bot
pushed a commit
that referenced
this pull request
Jan 2, 2023
* fix: prevent emails from being queued multiple times * style: fmt * refactor: optimistic error handling - Combined email queue sending to single call instead of different branches for testing. - If failing to get jobs -> attempt sending anyway. Failure could be form one bad job which shouldn't completely stop everything. * chore: type Co-authored-by: Ankush Menat <ankush@frappe.io> (cherry picked from commit 6b0950e)
ankush
pushed a commit
that referenced
this pull request
Jan 2, 2023
* fix: prevent emails from being queued multiple times * style: fmt * refactor: optimistic error handling - Combined email queue sending to single call instead of different branches for testing. - If failing to get jobs -> attempt sending anyway. Failure could be form one bad job which shouldn't completely stop everything. * chore: type Co-authored-by: Ankush Menat <ankush@frappe.io> (cherry picked from commit 6b0950e) Co-authored-by: stephen <stephenBDT@users.noreply.github.com>
ankush
added a commit
that referenced
this pull request
Jan 2, 2023
* fix: prevent emails from being queued multiple times * style: fmt * refactor: optimistic error handling - Combined email queue sending to single call instead of different branches for testing. - If failing to get jobs -> attempt sending anyway. Failure could be form one bad job which shouldn't completely stop everything. * chore: type Co-authored-by: Ankush Menat <ankush@frappe.io> (cherry picked from commit 6b0950e)
ankush
pushed a commit
that referenced
this pull request
Jan 2, 2023
* fix: prevent emails from being queued multiple times * style: fmt * refactor: optimistic error handling - Combined email queue sending to single call instead of different branches for testing. - If failing to get jobs -> attempt sending anyway. Failure could be form one bad job which shouldn't completely stop everything. * chore: type Co-authored-by: Ankush Menat <ankush@frappe.io> (cherry picked from commit 6b0950e) Co-authored-by: stephen <stephenBDT@users.noreply.github.com>
frappe-pr-bot
pushed a commit
that referenced
this pull request
Jan 4, 2023
## [13.46.3](v13.46.2...v13.46.3) (2023-01-04) ### Bug Fixes * only allow system manager to create/edit email templates by default (backport [#19458](#19458)) ([4dedbd8](4dedbd8)) * prevent emails from being queued multiple times ([#19386](#19386)) ([#19453](#19453)) ([4b1ad20](4b1ad20)) * **socketio:** Event list_update > doctype_subscribe (backport [#19422](#19422)) ([#19431](#19431)) ([1a2a1cc](1a2a1cc))
frappe-pr-bot
pushed a commit
that referenced
this pull request
Jan 4, 2023
## [14.21.1](v14.21.0...v14.21.1) (2023-01-04) ### Bug Fixes * Allow everyone to read geo data ([#19451](#19451)) ([#19457](#19457)) ([54eecce](54eecce)) * build_response for re.Match ([#19442](#19442)) ([c3729c7](c3729c7)) * don't set default in list view for fields not allowed ([373234d](373234d)) * **link/js:** handle undefined `link_title_doctypes` in `frappe.boot` ([#19398](#19398)) ([#19402](#19402)) ([f314e58](f314e58)) * only allow system manager to create email templates ([f46a649](f46a649)) * prevent emails from being queued multiple times ([#19386](#19386)) ([#19454](#19454)) ([21b84a4](21b84a4)) * show document title instead of name in like notification email subject ([41af70d](41af70d)) * **socketio:** Event list_update > doctype_subscribe (backport [#19422](#19422)) ([#19432](#19432)) ([4529e6d](4529e6d)) * **test:** override fields of test doctype if passed ([8a86a8c](8a86a8c)) * use title instead of name in message body too ([5878b53](5878b53))
stephenBDT
added a commit
to alias/frappe
that referenced
this pull request
Jan 10, 2023
…rappe#19454) * fix: prevent emails from being queued multiple times * style: fmt * refactor: optimistic error handling - Combined email queue sending to single call instead of different branches for testing. - If failing to get jobs -> attempt sending anyway. Failure could be form one bad job which shouldn't completely stop everything. * chore: type Co-authored-by: Ankush Menat <ankush@frappe.io> (cherry picked from commit 6b0950e) Co-authored-by: stephen <stephenBDT@users.noreply.github.com>
stephenBDT
pushed a commit
to alias/frappe
that referenced
this pull request
Jan 10, 2023
## [14.21.1](frappe/frappe@v14.21.0...v14.21.1) (2023-01-04) ### Bug Fixes * Allow everyone to read geo data ([frappe#19451](frappe#19451)) ([frappe#19457](frappe#19457)) ([54eecce](frappe@54eecce)) * build_response for re.Match ([frappe#19442](frappe#19442)) ([c3729c7](frappe@c3729c7)) * don't set default in list view for fields not allowed ([373234d](frappe@373234d)) * **link/js:** handle undefined `link_title_doctypes` in `frappe.boot` ([frappe#19398](frappe#19398)) ([frappe#19402](frappe#19402)) ([f314e58](frappe@f314e58)) * only allow system manager to create email templates ([f46a649](frappe@f46a649)) * prevent emails from being queued multiple times ([frappe#19386](frappe#19386)) ([frappe#19454](frappe#19454)) ([21b84a4](frappe@21b84a4)) * show document title instead of name in like notification email subject ([41af70d](frappe@41af70d)) * **socketio:** Event list_update > doctype_subscribe (backport [frappe#19422](frappe#19422)) ([frappe#19432](frappe#19432)) ([4529e6d](frappe@4529e6d)) * **test:** override fields of test doctype if passed ([8a86a8c](frappe@8a86a8c)) * use title instead of name in message body too ([5878b53](frappe@5878b53))
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We had problems sending out emails because of wrong credentials which made the sending of emails go into a timeout (which took a while)
the frappe.email.queue.flush runs every ~5 minutes but our emails were not nearly all sent which led to a HUGE job-queue, because the flush email jobs would see the same Email Queue documents again and again and always queue another sending job....
This attempts to fix this.