With this patch I'm trying to introduce a file-type object in logger, which could swallow everything thrown to stderr (and possibly stdout) and redirect to python logger. Python smtplib debug mode prints everything to stderr, but when wallace runs...
ClosedPublic

Authored by adomaitis on Mar 15 2018, 3:18 PM.

Diff Detail

Repository
rP pykolab
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
adomaitis created this revision.Mar 15 2018, 3:18 PM
adomaitis updated this revision to Diff 1309.Mar 15 2018, 3:20 PM

This is using the new file-like object from pykolab.logger. With this patch smtplib debug output gets redirected to logger rather than to stderr, which is not available in fork mode

adomaitis updated this revision to Diff 1312.Mar 15 2018, 3:23 PM

This amendment tries to avoid cyruslib printing to non-available file descriptor, which causes traceback of wallace.

machniak added inline comments.Mar 15 2018, 3:47 PM
pykolab/imap/cyrus.py
147–153

Looks like indentation here is broken.

adomaitis updated this revision to Diff 1318.Mar 15 2018, 4:21 PM

In addition LDAP trace_file needs to be specified. It is a file-like object define in logger.py Now we get ldap debug logs in pykolab.log file

adomaitis updated this revision to Diff 1321.Mar 15 2018, 4:27 PM

Fixing intendation

adomaitis updated this revision to Diff 1327.Mar 16 2018, 10:38 AM

Additionally, making wallace log.exception to log tracebacks

adomaitis updated this revision to Diff 1333.Mar 16 2018, 11:30 AM

I think we need process id (PID) logged for any level log record, that makes it easy to associate and track wallace and other pykolab modules work flow.

adomaitis updated this revision to Diff 1339.Mar 16 2018, 12:08 PM

Fixing another wrong intendation

adomaitis updated this revision to Diff 1345.Mar 18 2018, 5:03 PM

Specifying a bit more detailed log message.

adomaitis updated this revision to Diff 1351.Mar 18 2018, 10:11 PM

Need to catch imaplib debug output as well

adomaitis updated this revision to Diff 1357.Mar 21 2018, 4:02 PM

Fixing previous mistake. Need that for messages 9 and lower to be logged.

adomaitis updated this revision to Diff 1363.Mar 27 2018, 3:00 PM

It turns out smtplib debug logging redirected to logger produces excessive new lines. This is attempt to mitigate the problem, make logs more pretty.

adomaitis updated this revision to Diff 1367.May 8 2018, 1:20 PM

Make invitationpolicy module use smtplib sendmail wrapper _sendmail from modules, which now supports debug level 9 logging.

adomaitis updated this revision to Diff 1373.May 8 2018, 1:37 PM

Make all pykolab log messages appear on level <8. This will isolate pykolab debug messages from 3rd partty libraries debug messages. -d debug -l 8 will print pykolab debug messages only, while -l debug -d 9 will print 3rd party libraries messages in addition to level 8 pykolab messages.

Looking good, except for the corner-case of an errmsg over an un-"captured" exception.

pykolab/imap/cyrus.py
152

I think you may need to;

except Exception, errmsg:

before you can refer to errmsg in the call to log.exception().

In send_update_notification() we send mail in a loop trying up to 5 times if an error occurrs. But this is the only place where we do this. I think it would make sense to do this in all cases, i.e. in _sendmail(). I see also we don't call smtp.quit() consistently. Once we do this on success only, once we do this on error too.

In D577#6587, @machniak wrote:

In send_update_notification() we send mail in a loop trying up to 5 times if an error occurrs. But this is the only place where we do this. I think it would make sense to do this in all cases, i.e. in _sendmail(). I see also we don't call smtp.quit() consistently. Once we do this on success only, once we do this on error too.

The calls to modules._sendmail() are themselves looped, by the caller. Does that address the concern?

adomaitis updated this revision to Diff 1379.May 8 2018, 4:03 PM

Eliminating inconsistensies of sending mail from wallace. Now all the logic is in modules._sendmail() wrapper. Invitationpolicy module is using that wrapper and makes it responsible for re-submistion logic.

adomaitis updated this revision to Diff 1382.May 8 2018, 4:05 PM

Fix conner case exception handling.

A few more remarks.

wallace/modules.py
157

This line would prevent smtp.quit() and other cleanup from being called.

160–164

Note that the existing smtplib.SMTP() used to be rebuilt the connection for here.

164

This does not invoke recreating the smtplib.SMTP() instance, therefore possibly looping?

machniak added inline comments.May 9 2018, 7:03 AM
wallace/module_invitationpolicy.py
1350

I'd put this log.debug() after modules._sendmail().

adomaitis updated this revision to Diff 1388.May 9 2018, 11:52 AM

Log debug message after _sendmail.

adomaitis updated this revision to Diff 1394.May 9 2018, 12:51 PM

yet another attempt to create a better wrapper for smtplib.sendmail(). Now instantiation of smtplib object and connect are separated.

vanmeeuwen accepted this revision.May 17 2018, 8:41 AM
This revision is now accepted and ready to land.May 17 2018, 8:41 AM
This revision was automatically updated to reflect the committed changes.