I'm starting a new thread, because the original one got hijacked by the
doctest/unittest flamefest.
I appear to have unsubscribed from zope3-checkins a while ago, which is
why I'm posting here.
Disclaimer: I'm not very good at doing code reviews. I tend to mention
every little detail that could've been improved and forget to praise
everything that's already perfect.
There are two checkins on the branch.
=
rev 84645
=
http://svn.zope.org/zope.sendmail/branches/grantma-retryfixes/?rev=84645view=rev
Matthew, Subversion thinks the two PDF files you added are plain-text
(svn:eol-style is native, and svn diff shows the actual contents of
the PDFs, which isn't very useful to human readers). I would suggest
svn propdel svn:eol-style *.pdf
svn propset svn:mime-type application/pdf *.pdf
Also, the Quue_*.pdf seems to have a typo in the file name.
At first sight those diagrams look scary and complicated. I'd look into
ways to express their content in a text file with plain-text
explanations and ASCII-art diagrams.
=
rev 84551
=
http://svn.zope.org/zope.sendmail/branches/grantma-retryfixes/?rev=84551view=rev
Index: src/zope/sendmail/configure.zcml
===
--- src/zope/sendmail/configure.zcml (revision 84550)
+++ src/zope/sendmail/configure.zcml (revision 84551)
@@ -14,11 +14,18 @@
!--
To send mail, uncomment the following directive and be sure to
-create the queue directory.
+create the queue directory.
+
+Other parameters sepcify the polling interval, and the retry
sepcify?
+interval used when a temporary failure is detected. Lock links
+can also be cleared on server start.
mail:queuedDelivery permission=zope.SendMail
queuePath=./queue
- mailer=smtp /
+ retryInterval=300
+ pollingInterval=3000
+ cleanLockLinks=False
+ mailer=smtp /
--
Please avoid literal tab characters in source files.
Index: src/zope/sendmail/zcml.py
===
--- src/zope/sendmail/zcml.py (revision 84550)
+++ src/zope/sendmail/zcml.py (revision 84551)
@@ -70,7 +70,35 @@ class IQueuedDeliveryDirective(IDelivery
description=uDefines the path for the queue directory.,
required=True)
-def queuedDelivery(_context, permission, queuePath, mailer, name=Mail):
+pollingInterval = Int(
+title=u'Polling Interval',
+description=u'Defines the polling interval for queue processing in'
+' milliseconds',
+default=3000,
+required=False)
...
-thread = QueueProcessorThread()
+thread = QueueProcessorThread(interval=pollingInterval/1000,
You're using integer division here and losing sub-second precision. If you
don't want that, write interval=pollingInterval/1000.0.
+ retry_interval=retryInterval,
+ clean_lock_links=cleanLockLinks)
thread.setMailer(mailerObject)
thread.setQueuePath(queuePath)
thread.start()
Index: src/zope/sendmail/mailer.py
===
--- src/zope/sendmail/mailer.py (revision 84550)
+++ src/zope/sendmail/mailer.py (revision 84551)
...
@@ -41,22 +49,105 @@ class SMTPMailer(object):
self.password = password
self.force_tls = force_tls
self.no_tls = no_tls
+self.logger = None
-def send(self, fromaddr, toaddrs, message):
-connection = self.smtp(self.hostname, str(self.port))
+def set_logger(self, logger):
+if not isinstance(logger, logging.Logger):
+raise DoesNotImplement('Invalid logger given')
I don't think DoesNotImplement is right here -- there are no interfaces
you're looking for. Better use the built-in TypeError.
But is the type check necessary at all? The Pythonic thing is to rely on duck
typing.
+self.logger = logger
+
+def _log(self, log_level, *args):
+if self.logger is None:
+return
+# This is not elegant, but it can be fixed later
In my experience later usually doesn't happen. Why not fix it now?
+if log_level == 'debug':
+self.logger.debug(*args)
+elif log_level =='info':
+self.logger.info(*args)
+elif log_level =='error':
+self.logger.error(*args)
+elif log_level =='warning':
+self.logger.warning(*args)
+
Just use
self.logger.log(log_level, *args)
and logging.DEBUG/logging.INFO/logging.ERROR/logging.WARNING as the
values for log_level.
+def _handle_smtp_error(self,
+ smtp_code,
+ smtp_error,
+
