Offer the possibility to keep a finite number of mails #375
Conversation
marcmillien
force-pushed
the
offer-the_possibility_to_keep_a_finite_number_of_mails
branch
2 times, most recently
from
3da0505
to
74cba52
Compare
marcmillien
force-pushed
the
offer-the_possibility_to_keep_a_finite_number_of_mails
branch
from
74cba52
to
1c6781e
Compare
lib/mail_catcher/mail.rb
Outdated
| @@ -157,4 +157,13 @@ def delete_message!(message_id) | |||
| @delete_messages_query.execute(message_id) and | |||
| @delete_message_parts_query.execute(message_id) | |||
| end | |||
|
|
|||
| def delete_older_messages!(max_mail_to_keep = MailCatcher::options[:max_mail_to_keep]) | |||
| return if max_mail_to_keep == -1 | |||
Good catch, I fix it :) |
Signed-off-by: Marc Millien <marc.millien@gmail.com>
marcmillien
force-pushed
the
offer-the_possibility_to_keep_a_finite_number_of_mails
branch
from
1c6781e
to
e884fe4
Compare
|
Can we get this merged please? @sj26 |
| def delete_older_messages!(max_mail_to_keep = MailCatcher::options[:max_mail_to_keep]) | ||
| return if max_mail_to_keep <= 0 | ||
| @delete_older_messages_query ||= db.prepare "DELETE FROM message WHERE id NOT IN (SELECT id FROM message ORDER BY created_at DESC LIMIT ?)" | ||
| @delete_older_message_parts_query ||= db.prepare "DELETE FROM message_part WHERE id NOT IN (SELECT id FROM message_part ORDER BY created_at DESC LIMIT ?)" |
There was a problem hiding this comment.
Are the right records getting deleted here?
Should this delete the same rows as the previous line?
DELETE from message_part
WHERE message_id NOT IN (SELECT id from message ORDER BY created_at DESC LIMIT ?)(or ORDER BY id if the previous ORDER BY change was made)
There was a problem hiding this comment.
You are right, I change this :).
|
|
||
| def delete_older_messages!(max_mail_to_keep = MailCatcher::options[:max_mail_to_keep]) | ||
| return if max_mail_to_keep <= 0 | ||
| @delete_older_messages_query ||= db.prepare "DELETE FROM message WHERE id NOT IN (SELECT id FROM message ORDER BY created_at DESC LIMIT ?)" |
There was a problem hiding this comment.
Since id is the primary key, it has an index. Can we ORDER BY id?
Also, dealing with just a single indexed column, most databases run much quicker, avoiding hitting the data page altogether.
It may be just a marginal savings though. So feel free to ignore
inspired by (stolen from) sj26#375
inspired by (stolen from) sj26#375
|
Over a year later and still not merged. Is there a reason for this not to be merged, or is the project abandoned? |
|
Thank you for your contribution @marcmillien! I'm truly sorry I haven't commented here yet. I like this feature. I didn't merge it because every time I looked at it, I realised it was predicated on a bunch of testing and docker changes, and teasing them apart was tricky enough to become "later" problem, and then later never came. Honestly this project is in a maintenance-ish mode — it works, and I'll keep it working, but I consider it feature-complete, and most of the things I would like to address require a complete rewrite which is a bit too monumental. Every time I come to look at this repo, I'm reminded of this, too. So, for right now, I'll rebase this, make a few adjustments, and then merge it. |
…p_a_finite_number_of_mails Offer the possibility to keep a finite number of mails
|
This has been rebased and merged — thanks! |
|
Please try v0.8.0.beta1 via |
|
This is shipped in the latest version, v0.8.0. |
Context
When
mailcatcherreceives thousand of mails per day the web interface is very slow and can take several minutes to appear.Description
This PR adds an option that allows to specify a max number of mail to keep, so
mailcatcheris responsive when people access it even in case of high catching trafic.References
Requires #374 before merge