Рефакторинг входящего poplib с использованием windows python 2.3 - PullRequest
1 голос
/ 22 октября 2008

Привет, ребята, не могли бы вы помочь мне реорганизовать это так, чтобы оно было явно питоническим.

import sys
import poplib
import string
import StringIO, rfc822
import datetime
import logging

def _dump_pop_emails(self):
    self.logger.info("open pop account %s with username: %s" % (self.account[0], self.account[1]))
    self.popinstance = poplib.POP3(self.account[0])
    self.logger.info(self.popinstance.getwelcome()) 
    self.popinstance.user(self.account[1])
    self.popinstance.pass_(self.account[2])
    try:
        (numMsgs, totalSize) = self.popinstance.stat()
        for thisNum in range(1, numMsgs+1):
            (server_msg, body, octets) = self.popinstance.retr(thisNum)
            text = string.join(body, '\n')
            mesg = StringIO.StringIO(text)                               
            msg = rfc822.Message(mesg)
            name, email = msg.getaddr("From")
            emailpath = str(self._emailpath + self._inboxfolder + "\\" + email + "_" + msg.getheader("Subject") + ".eml")
            emailpath = self._replace_whitespace(emailpath)
            file = open(emailpath,"wb")
            file.write(text)
            file.close()
            self.popinstance.dele(thisNum)
    finally:
        self.logger.info(self.popinstance.quit())

def _replace_whitespace(self,name):
    name = str(name)
    return name.replace(" ", "_")   

Также в методе _replace_whitespace я хотел бы иметь какую-то процедуру очистки, которая удаляет все недопустимые символы, которые могут вызвать обработку.

В основном я хочу написать письмо в папку входящих сообщений стандартным способом.

Я что-то здесь не так делаю?

Ответы [ 3 ]

3 голосов
/ 22 октября 2008

Я не вижу в этом коде ничего существенного неправильного - он ведет себя неправильно или вы просто ищете общие рекомендации по стилю?

Несколько заметок:

  1. Вместо logger.info ("foo %s %s" % (bar, baz)) используйте "foo %s %s", bar, baz. Это позволяет избежать затрат на форматирование строки, если сообщение не будет напечатано.
  2. Положите try...finally вокруг отверстия emailpath.
  3. Используйте '\n'.join (body) вместо string.join (body, '\n').
  4. Вместо msg.getaddr("From"), просто msg.From.
1 голос
/ 22 октября 2008

Это не рефакторинг (насколько я вижу, он не требует рефакторинга), но некоторые предложения:

Вы должны использовать пакет электронной почты, а не rfc822. Замените rfc822.Message на email.Message и используйте email.Utils.parseaddr (msg ["From"]), чтобы получить имя и адрес электронной почты, и msg ["Subject"], чтобы получить тему.

Используйте os.path.join для создания пути. Это:

emailpath = str(self._emailpath + self._inboxfolder + "\\" + email + "_" + msg.getheader("Subject") + ".eml")

становится:

emailpath = os.path.join(self._emailpath + self._inboxfolder, email + "_" + msg.getheader("Subject") + ".eml")

(Если self._inboxfolder начинается с косой черты или self._emailpath заканчивается на 1, вы можете заменить первый + на запятую).

Это на самом деле ничего не ранит, но вам, вероятно, не следует использовать «файл» в качестве имени переменной, поскольку он скрывает встроенный тип (шашки, такие как pylint или pychecker, предупредят вас об этом).

Если вы не используете self.popinstance вне этой функции (кажется маловероятным, учитывая, что вы подключаетесь и выходите из функции), то нет смысла делать его атрибутом self. Просто используйте «popinstance».

Использовать xrange вместо range.

Вместо того, чтобы просто импортировать StringIO, сделайте это:

try:
    import cStringIO as StringIO
except ImportError:
    import StringIO

Если это почтовый ящик POP, к которому могут обращаться несколько клиентов одновременно, вы можете захотеть сделать попытку / исключение вокруг вызова RETR, чтобы продолжить, если вы не можете получить одно сообщение.

Как сказал Джон, используйте "\ n" .join вместо string.join, используйте try / finally, чтобы закрыть файл, только если он открыт, и отдельно передать параметры ведения журнала.

Единственная проблема с рефакторингом, о которой я мог подумать, заключается в том, что вам на самом деле не нужно анализировать все сообщение, поскольку вы просто сбрасываете копию необработанных байтов, а вам нужны только заголовки From и Subject. Вместо этого вы можете использовать popinstance.top (0), чтобы получить заголовки, создать из него сообщение (пустое тело) и использовать его для заголовков. Затем выполните полное RETR, чтобы получить байты. Это стоило бы сделать, только если ваши сообщения были большими (поэтому их анализ занимал много времени). Я определенно измерил бы, прежде чем я сделал эту оптимизацию.

От того, что ваша функция очищает для имен, зависит, насколько хороши вы хотите, чтобы имена были, и насколько вы уверены, что электронная почта и тема делают имя файла уникальным (кажется довольно маловероятным). Вы можете сделать что-то вроде:

emailpath = "".join([c for c in emailpath if c in (string.letters + string.digits + "_ ")])

И в итоге вы получите только буквенно-цифровые символы, знак подчеркивания и пробел, которые кажутся читаемыми. Учитывая, что ваша файловая система (с Windows), вероятно, нечувствительна к регистру, вы также можете использовать строчные буквы (добавьте .lower () в конец) Вы можете использовать emailpath.translate, если хотите что-то более сложное.

0 голосов
/ 22 октября 2008

В дополнение к моему комментарию к ответу Джона

Я выяснил, в чем проблема, в поле имени и поле темы были недопустимые символы, из-за чего python получал икоты, когда он пытался записать письмо в виде каталога, увидев ":" и "/ ».

Джон Точка № 4 не работает! поэтому я оставил это как прежде. Также верно ли пункт № 1, правильно ли я выполнил ваше предложение?

def _dump_pop_emails(self):
    self.logger.info("open pop account %s with username: %s", self.account[0], self.account[1])
    self.popinstance = poplib.POP3(self.account[0])
    self.logger.info(self.popinstance.getwelcome()) 
    self.popinstance.user(self.account[1])
    self.popinstance.pass_(self.account[2])
    try:
        (numMsgs, totalSize) = self.popinstance.stat()
        for thisNum in range(1, numMsgs+1):
            (server_msg, body, octets) = self.popinstance.retr(thisNum)
            text = '\n'.join(body)
            mesg = StringIO.StringIO(text)                               
            msg = rfc822.Message(mesg)
            name, email = msg.getaddr("From")
            emailpath = str(self._emailpath + self._inboxfolder + "\\" + self._sanitize_string(email + " " + msg.getheader("Subject") + ".eml"))
            emailpath = self._replace_whitespace(emailpath)
            print emailpath
            file = open(emailpath,"wb")
            file.write(text)
            file.close()
            self.popinstance.dele(thisNum)
    finally:
        self.logger.info(self.popinstance.quit())

def _replace_whitespace(self,name):
    name = str(name)
    return name.replace(" ", "_")   

def _sanitize_string(self,name):
    illegal_chars = ":", "/", "\\"
    name = str(name)
    for item in illegal_chars:
        name = name.replace(item, "_")
    return name
...