2
\$\begingroup\$

This is my first experience of creating a useful tool in Python. I'd appreciate any critics on this.
I can also post the config file if needed.

# !/usr/bin/env python
#  -*- coding: utf-8 -*-

"""
A Python script that checks for Portage tree and overlays updates and notifies
of the list of packages available for upgrade.
This tool was inspired by Porticron (https://github.com/hollow/porticron)
by Benedikt Böhm.
"""

import os
import subprocess
import smtplib
import socket
import ConfigParser
import xmpp
import re
import tempfile
import time


COLORS = {'\x1b[32m': '</span><span style="color:darkgreen;font-weight:normal">',
          '\x1b[36;01m': '</span><span style="color:turquoise;font-weight:bold">',
          '\x1b[34;01m': '</span><span style="color:blue;font-weight:bold">',
          '\x1b[39;49;00m': '</span><span style="color:black;font-weight:normal">',
          '\x1b[33;01m': '</span><span style="color:orange;font-weight:bold">',
          '\x1b[32;01m': '</span><span style="color:limegreen;font-weight:bold">',
          '\x1b[31;01m': '</span><span style="color:red;font-weight:bold">',
          '\x0d': '<br>'
          }


class Timestamp(tempfile._RandomNameSequence):
    def next(self):
        return time.strftime('%Y%m%d-%H%M%S')

tempfile._RandomNameSequence = Timestamp

class GUN():
    """Main class
    """

    def __init__(self):
        """Reads configuration file, performs basic setup
        """
        config = ConfigParser.RawConfigParser()
        try:
            config.read('/etc/gun.conf')
        except IOError:
            print 'Cannot open configuration file'

        # General settings
        self.sync_overlays = config.getboolean('GENERAL', 'sync_overlays')

        # Synchronization settings
        self.sync_tree_command = config.get('SYNC', 'tree_command')
        if self.sync_overlays:
            self.sync_overlays_command = config.get('SYNC', 'overlays_command')
        emerge_command = config.get('UPDATE', 'command')

        # Notification settings
        self.email_notify = config.getboolean('NOTIFY', 'email')
        self.jabber_notify = config.getboolean('NOTIFY', 'jabber')

        # Email settings
        self.mail_host = config.get('EMAIL', 'host')
        self.mail_user = config.get('EMAIL', 'user')
        self.mail_password = config.get('EMAIL', 'password')
        self.mail_port = config.get('EMAIL', 'port')
        self.mail_sender = config.get('EMAIL', 'mailfrom')
        self.mail_recipient = config.get('EMAIL', 'mailto')

        # Jabber settings
        self.jabber_sender = config.get('JABBER', 'jabber_from')
        self.jabber_password = config.get('JABBER', 'password')
        self.jabber_recipient = config.get('JABBER', 'jabber_to')

        # Output file
        timestamp = time.strftime('%Y%m%d-%H%M%S')
        self.output_file = tempfile.NamedTemporaryFile(suffix = '',
                                                       prefix = 'gun-',
                                                       dir = '/var/tmp',
                                                       delete = False)

        self.update_command = 'script -q -c \'%s\' -f %s' % (emerge_command,
                                                             self.output_file.name)

    def sync(self):
        """SYNC Portage tree and overlays
        """
        output = subprocess.Popen([self.sync_tree_command],
                                  shell=True,
                                  stdout=subprocess.PIPE)
        output.communicate()
        if self.sync_overlays:
            output = subprocess.Popen([self.sync_overlays_command],
                                      shell=True,
                                      stdout=subprocess.PIPE)
            output.communicate()

    def pretend_update(self):
        """Runs emerge to get the list of packages ready for upgrade.
        Writes the list to file.
        """
        output = subprocess.Popen([self.update_command],
                                  shell=True)
        output.communicate()
        self.output_file.seek(0)

    def _ansi2html(self, colors):
        """Parses the output file, produced by the script Linux tool and
        translates ANSI terminal color codes into HTML tags
        """
        # Remove 'header' and 'footer' from the file
        text = re.sub("(?s).*?(\[\x1b\[32mebuild)",
                      "\\1",
                      self.output_file.read(),
                      1).split('Total', 1)[0]
        pattern = '|'.join(map(re.escape, colors.keys()))
        message = re.sub(pattern,
                         lambda m:colors[m.group()],
                         text)
        self.output_file.seek(0)

        return message

    def _send_email(self):
        headers = ("From: %s\r\nTo: %s\r\nSubject: [%s] %s Packages to update\r\nContent-Type: text/html; charset=utf-8\r\n"
                   % (self.mail_sender,
                      self.mail_recipient,
                      os.uname()[1],
                      time.strftime('%Y-%m-%d')))
        message = headers + self._ansi2html(colors = COLORS)
        server = smtplib.SMTP()
        try:
            server.connect(self.mail_host,
                            self.mail_port)
        except socket.error:
            print 'Cannot connect to SMTP server'
        try:
            server.login(self.mail_user,
                         self.mail_password)
        except smtplib.SMTPAuthenticationError:
            print 'Cannot authenticate on SMTP server'
        server.sendmail(self.mail_sender,
                        self.mail_recipient,
                        message)
        server.quit()

    def _send_jabber(self):
        for key in COLORS.iterkeys():
            COLORS[key] = ''
        message = self._ansi2html(colors = COLORS)
        jid = xmpp.protocol.JID(self.jabber_sender)
        cl = xmpp.Client(jid.getDomain(),
                         debug=[])
        if not cl.connect():
            raise IOError('Cannot connect to Jabber server')
        else:
            if not cl.auth(jid.getNode(),
                           self.jabber_password,
                           resource=jid.getResource()):
                raise IOError('Cannot authenticate on Jabber server')
            else:
                cl.send(xmpp.protocol.Message(self.jabber_recipient,
                                              message))
        cl.disconnect()

    def notify(self):
        if self.email_notify:
            self._send_email()
        if self.jabber_notify:
            self._send_jabber()


def main():
    s = GUN()
    s.sync()
    s.pretend_update()
    s.notify()

# EOF
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
class Timestamp(tempfile._RandomNameSequence):
    def next(self):
        return time.strftime('%Y%m%d-%H%M%S')

tempfile._RandomNameSequence = Timestamp

Names beginning with underscores are considered internal to that module. Monkey patching like this is generally a hack which should be avoided. Since you are producing the string based on the current time (and only up to milliseconds), this class you are putting in probably doesn't act the way the tempfile code expects it to.

class GUN():

Use class Gun: or class Gun(object): If this python 2.x the later is better.

    def __init__(self):
        """Reads configuration file, performs basic setup
        """
        config = ConfigParser.RawConfigParser()
        try:
            config.read('/etc/gun.conf')
        except IOError:
            print 'Cannot open configuration file'

Stylistically, I'd pass the config object to the constructor. That way I've got more flexibility if I want to reuse the object.

        # Output file
        timestamp = time.strftime('%Y%m%d-%H%M%S')
        self.output_file = tempfile.NamedTemporaryFile(suffix = '',
                                                       prefix = 'gun-',
                                                       dir = '/var/tmp',
                                                       delete = False)

What exactly is the point of using tempfile here? Given that you've replaced the random sequence with your own, why don't you just create the file? Why do want to control the name of the file anyways?

        self.update_command = 'script -q -c \'%s\' -f %s' % (emerge_command,
                                                             self.output_file.name)

Another option to look at is using subprocess and reading the standard_output into memory.

    def sync(self):
        """SYNC Portage tree and overlays
        """
        output = subprocess.Popen([self.sync_tree_command],
                                  shell=True,
                                  stdout=subprocess.PIPE)
        output.communicate()

This piece of code modulo the command executed happens over and over and over again in your code. Write a helper function.

    def _ansi2html(self, colors):
        """Parses the output file, produced by the script Linux tool and
        translates ANSI terminal color codes into HTML tags
        """
        # Remove 'header' and 'footer' from the file
        text = re.sub("(?s).*?(\[\x1b\[32mebuild)",
                      "\\1",
                      self.output_file.read(),
                      1).split('Total', 1)[0]
        pattern = '|'.join(map(re.escape, colors.keys()))
        message = re.sub(pattern,
                         lambda m:colors[m.group()],
                         text)
        self.output_file.seek(0)

        return message

Stylistically, I'd move this to be a free standing function, and pass in the output_file.

    def _send_email(self):
        headers = ("From: %s\r\nTo: %s\r\nSubject: [%s] %s Packages to update\r\nContent-Type: text/html; charset=utf-8\r\n"
                   % (self.mail_sender,
                      self.mail_recipient,
                      os.uname()[1],
                      time.strftime('%Y-%m-%d')))
        message = headers + self._ansi2html(colors = COLORS)
        server = smtplib.SMTP()
        try:
            server.connect(self.mail_host,
                            self.mail_port)
        except socket.error:
            print 'Cannot connect to SMTP server'
        try:
            server.login(self.mail_user,
                         self.mail_password)
        except smtplib.SMTPAuthenticationError:
            print 'Cannot authenticate on SMTP server'
        server.sendmail(self.mail_sender,
                        self.mail_recipient,
                        message)
        server.quit()

Stylistically, sending e-mail is distinct from what this class is doing. I'd probably look into moving this into its own class. So I'd have an e-mail sending class which this would call with the data.

    def notify(self):
        if self.email_notify:
            self._send_email()
        if self.jabber_notify:
            self._send_jabber()

What I'd do here is implement email sending and jabber sending as two objects which both implement the same interface. Then notify would loop over a list of notifiables which would be those two objects. Then you'd reduce coupling because GUN wouldn't know about e-mail/jabber or vice versa.

def main():
    s = GUN()
    s.sync()
    s.pretend_update()
    s.notify()

Typically something like

if __name__ == '__main__':
      main()

Is included so that python will execute the main function when the script is run.

# EOF

That's a pretty much useless comment.

EDIT

Notify structure:

class JabberNotifier(object):
    def __init__(self, config)
        # get jabber config settings

    def send(self, message):
        ...

class EmailNotifier(object):
    def __init__(self, config)
       # get email config settings

    def send(self, message):
       ...

class GUN(object):
    def __init__(self, config):
       self.notifiers = []
       if config.should_do_emails():
            self.notifiers.append( EmailNotifier(config) )
       if config.should_do_jabber():
            self.notifiers.append( JabberNotifier(config) )

   def notify(self):
       for notifier in self.notifiers:
            notifier.send(message)

The real purist in me would also be inclined to move config stuff completely out of the constructors. Basically, I'd pass all of the parameters that you are pulling out the config file into the constructors.

Also, you can do the same thing that script is doing inside python using the pty module. If you use that you should be able to avoid using script or hitting temporary files.

\$\endgroup\$
12
  • \$\begingroup\$ 1. Regarding the tempfile module patching and using - this was just an attempt to learn how to patch modules in runtime. I'll consider creating a simple file instead of using the tempfile module. 2. Regarding reading the standard_output into memory. The reason why I am using the script linux tool is that I need the output with terminal colors, which I then translate into HTML. Reading to stdout strips all the formatting (colors in particular). And I could not find any Pythonic approach to this. As for the rest of remarks - thanks, I'll change the script accordingly. \$\endgroup\$ Sep 19, 2011 at 14:22
  • \$\begingroup\$ And the main function is called from the executable file, which is created with setup.py: entry_points = { 'console_scripts': [ 'gun = gun.sync:main' ] \$\endgroup\$ Sep 19, 2011 at 14:31
  • \$\begingroup\$ @Ch00k, I'm somewhat surprised by the colors. Its not unusual for a program to not send colors out when its not actually on a tty. But then I'd expect not to write it to your file either. \$\endgroup\$ Sep 19, 2011 at 18:05
  • \$\begingroup\$ I do need the colors, that's why I've chosen to use script. It can produce the same output for a non-tty program, as if it was run in a tty. \$\endgroup\$ Sep 19, 2011 at 19:04
  • \$\begingroup\$ I somewhat reworked my script according to your suggestions. However, I still cannot understand the notify method. What exactly do you mean? Maybe you could provide an example for me to better understand? \$\endgroup\$ Sep 19, 2011 at 19:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.