Hi
Ramdas S
Where do we start?
RS
James Tauber
(1) Check out the latest code and work through the README. Let me
know if you have any feedback on this low layer.
(2) Look at any parts of the code I've marked @@@
(3) Let me know if you have any feedback on my initial brain dump at:
http://code.google.com/p/django-mailer/wiki/InitialDesignThoughts
(the first half I've already implemented and checked in, so mostly
the second half)
Jökull Auðunsson
I'm not sure I follow you on "Don't send list". In my email model I have email and subscriber status (inactive for unsubscribed). These, obviously, get cut out of any email sending.
Why not just put a status message in your queue model that would serve as a log. If there is an error it's marked as "not sent" in status and with an error message from the SMTP dispatcher in the log field.
Do you want to see my code?
James Tauber
useless. I need to loop over all highs, check for more highs, then
move on to mediums and so on. Under the current implementation, a new
high will not jump ahead.
James
Sent from my iPhone
Jökull
I just read over some of the use cases on the project page. I like
scheduled sending and tying into the user model in Django. I'm not
sure about fine-grained user specific stuff let alone activity-based
sending (not for this project's code base anyway). User specific stuff
tends to be site specific. In my mind that definitely means that it
belongs in site specific code. BUT - it should be easy to tie these
site-specific mailings into django-mailer's Dispatch/Queue table so
that all email is managed in one spot and one cronjob. Does that mean
we need to have an "origin" field in Dispatch to separate mailings
from django-mailer and other site apps?
On Aug 8, 7:04 pm, James Tauber <jtau...@jtauber.com> wrote:
> It just occurred to me that, as currently implemented, priority is
> useless. I need to loop over all highs, check for more highs, then
> move on to mediums and so on. Under the current implementation, a new
> high will not jump ahead.
>
> James
>
> Sent from my iPhone
>
James Tauber
loop:
while exists high or exists medium:
while exists high:
loop over all high:
send message
while not exists high and exists medium:
get first medium
send message
while not exists high and not exists medium:
get first low
send message
James
James Tauber
> I'm not sure I see the use for a priority level.
Is it clearer now I've corrected the algorithm?
Here's a typical use case as I see it: You have a queue of 10,000
messages with throttling set for 10 message per second. You have 100
urgent messages you want to bump up to the start of the queue.
> I'm not sure I follow you on "Don't send list". In my email model I
> have email and subscriber status (inactive for unsubscribed).
> These, obviously, get cut out of any email sending.
Remember that the use cases for django-mailer extend beyond just a
subscription model. I guess one could argue that everything could be
modeled as subscription but I think at the lowest level, it's a more
flexible architecture to just have a "send this email to these users"
that is agnostic about why that email is being sent to those users.
I imagine wanting to use django-mailer even for one off individual
emails like the registration activation. Once support for it is done,
it will be useful to have django-mailer catch mail delivery failures
in that instance, for example.
With that background let me return to why I wanted a "Don't send
list"...
In my experience it's useful to have a quick "emergency stop"
capability. Someone emails you and says "don't send me email!" and
you want to just quickly prevent any more email from being sent to
them without having to go change queries or remove from multiple
subscription lists.
> Why not just put a status message in your queue model that would
> serve as a log. If there is an error it's marked as "not sent" in
> status and with an error message from the SMTP dispatcher in the
> log field.
Separating the log from the send queue seems cleaner to me. Otherwise
one is constantly applying an extra filter. My design is similar to
the database design pattern of moving "deleted" objects to a new
table rather than marking them deleted in the same table.
> Do you want to see my code?
Definitely. I see you've already posted it. Haven't looked yet.
James
Jökull Auðunsson
James Tauber
James Tauber
existence of low), I'm implemented this algorithm with the following
generator:
where XXX_priority() have been defined on the custom manager.
James
James Tauber
philp
First off, I'd like to say this is a great idea for a project, and
something which will prove very useful. It's something which is
regularly requested as a feature in projects I work on, so I'm keen to
help out wherever possible.
I interested to see this algorithm code, but have a query/suggestion:
I don't quite understand why you're doing so much iteration through
custom Managers, when it would seem to me that the place for dealing
with this logic already exists in Django. Perhaps I'm missing
something, but there are a heck of a lot of SQL queries being fired in
this code.
Rather than XXX_priority, you could use a objects_prioritized()
Manager which does the ordering within the model:
def objects_prioritized(self):
return self.objects.order_by('priority', 'when_added')
That would return you all Message objects, ordered by ascending
priority, then ordered by the time they were added. Your prioritize
method is using 12 SQL queries, where you only really need to use 1.
Just my thoughts - I may be missing some good reason as to why you're
doing it the way you are.
-Phil
James Tauber
Hi Phil and welcome!
Prior to r4, I did things exactly as you suggested. The problem with
a single SQL query is if a new high priority mail is added to the
queue during iteration over the results of that query, the existing
query set will all get sent out before the new hight priority mail is
even considered.
This is not what is wanted as, instead, we want high priority mail
added during iteration to "jump the queue" if we're iterating over
medium or low and we want medium priority mail added during iteration
to jump the queue if we're iterating over low priority.
One optimization I guess could still be made is, rather than yielding
just one non-high before checking if there are any new highs (and so
on), I could make that number configurable. So you could send at most
n medium mails before checking for highs, at most n low mails before
checks for highs or mediums.
This would mean replacing:
yield Message.objects.xxx_priority().order_by('when_added')[0]
with
for message in Message.objects.xxx_priority().order_by
('when_added')[:MAX_MAILS_BEFORE_CHECKING_FOR_HIGHER_PRIORITY]:
yield message
James
philp
humour me as I play devil's advocate a little more though:
I can see how you'd want to ensure that prioritized mail gets to the
head of the queue, but is there not a compromise to be made between
performance and how reactive the application is? I personally feel
that when the send_all() method is called, everything in the queue at
that moment in time is despatched.
I like to think of it like checking in for a plane: at set intervals,
the gate is shut, no more passengers can board, and the flight
departs. 30 seconds later, the next flight is ready to go. In the
interval between those flights, a new passenger list is put together,
with those which have highest priority jumping to the front of the
queue.
It seems to me that it's a little risky to be manipulating the queue
during the process of sending, when a simple marshall to manage what
is sent, and when it is sent, would be a more elegant (and speedy)
solution. Managing things in such a way would also have the advantage
of being able to easily manage simple throttling, to tell the
application "send 20 messages every 30 seconds".
Like I say: just more thoughts - please feel free to ignore me!
-Phil
James Tauber
If one is unlikely to have a queue that takes a long time to clear,
your suggestion (which was my original implementation) is good.