Réécriture de poplib entrante en utilisant Windows python 2.3

Question

Salut les gars, pourriez-vous s'il vous plait m'aider à refactoriser ceci pour qu'il soit sensiblement pythonique.

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(" ", "_") 

également dans la méthode _replace_whitespace Je voudrais avoir une sorte de routine de nettoyage qui prend tous les caractères illégaux qui pourraient causer le traitement.

Fondamentalement, je veux écrire le courriel dans le répertoire de la boîte de réception d'une manière standard.

Est-ce que je fais quelque chose de mal ici?

Answer 1

Ce n'est pas refactoring (il n'a pas besoin refactorisation pour autant que je peux voir), mais quelques suggestions:

Vous devez utiliser le logiciel de courrier électronique plutôt que rfc822. Remplacez rfc822.Message par email.Message, et utilisez email.Utils.parseaddr (msg ["De"]) pour obtenir le nom et l'adresse e-mail, et msg ["Subject"] pour obtenir le sujet.

Utilisez os.path.join pour créer le chemin. Ce:

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

Devient:

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

(Si self._inboxfolder commence par une barre oblique ou self._emailpath se termine par un, vous pouvez remplacer le premier par une virgule + aussi). Cela ne fait pas vraiment de mal, mais vous ne devriez probablement pas utiliser "file" comme nom de variable, car il ombrage un type intégré (les vérificateurs comme pylint ou pychecker vous préviennent à ce sujet).

Si vous n'utilisez pas self.popinstance en dehors de cette fonction (cela semble peu probable étant donné que vous vous connectez et que vous quittez la fonction), cela ne sert à rien d'en faire un attribut de self.Utilisez simplement "popinstance" par lui-même.

Utilisez xrange au lieu de gamme.

Au lieu de StringIO simplement importer, faites ceci:

try: 
    import cStringIO as StringIO 
except ImportError: 
    import StringIO 

Si cela est une boîte aux lettres POP qui peut être consulté par plus d'un client à la fois, vous voudrez peut-être mettre un try/except autour de la Appel RETR pour continuer si vous ne pouvez pas récupérer un message. Comme John l'a dit, utilisez "\ n" .join plutôt que string.join, utilisez try/finally pour fermer seulement le fichier s'il est ouvert, et passez les paramètres de journalisation séparément. Le problème de refactoring que je pourrais penser serait que vous n'avez pas vraiment besoin d'analyser le message entier, puisque vous êtes en train de jeter une copie des octets bruts, et tout ce que vous voulez est les en-têtes From et Subject . Vous pouvez à la place utiliser popinstance.top (0) pour obtenir les en-têtes, créer le message (corps vide) à partir de cela, et l'utiliser pour les en-têtes. Faites ensuite un RETR complet pour obtenir les octets. Cela ne vaudrait la peine d'être fait que si vos messages étaient volumineux (et donc les analyser prenait beaucoup de temps). Je voudrais certainement mesurer avant d'avoir fait cette optimisation. Pour que votre fonction désinfecte les noms, cela dépend de la manière dont vous voulez que les noms soient, et de la certitude que l'email et le sujet rendent le nom de fichier unique (cela semble assez improbable). Vous pouvez faire quelque chose comme:

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

Et vous finiriez avec seulement des caractères alphanumériques et le trait de soulignement et de l'espace, ce qui semble être un ensemble lisible. Étant donné que votre système de fichiers (avec Windows) est probablement insensible à la casse, vous pouvez aussi minuscules (ajoutez .lower() à la fin). Vous pouvez utiliser emailpath.translate si vous voulez quelque chose de plus complexe.

Answer 2

Je ne vois rien de grave avec ce code - se comporte-t-il de manière incorrecte ou recherchez-vous simplement des directives de style générales?

Quelques notes:

1. Au lieu de logger.info ("foo %s %s" % (bar, baz)), utilisez "foo %s %s", bar, baz. Cela évite la surcharge du formatage de chaîne si le message ne sera pas imprimé.
2. Mettez un try...finally autour de l'ouverture emailpath.
3. Utilisez '\n'.join (body), au lieu de string.join (body, '\n'). Au lieu de msg.getaddr("From"), juste msg.From.

Answer 3

suite à mon commentaire sur la réponse de John

J'ai trouvé ce que la question était, il y avait des caractères illégaux dans le champ Nom et champ Objet, qui a causé python pour obtenir le hoquet, comme il a essayé d'écrire l'email en tant que répertoire, après avoir vu ":" et "/".

John point numéro 4 ne fonctionne pas! donc je l'ai laissé comme avant. Est-ce que le point n ° 1 est correct? Est-ce que j'ai implémenté correctement votre suggestion?

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