Python Code Efficiency - For the Beginner

[Octet]

Hello Daniweb,

I'm learning Python at the moment, and I'm starting by looking at automating some of my server tasks, namely things like backups, virus scans and checking for IP changes (being on a dynamic IP, automating this task will be very helpful).

I've started by creating some code which I shall run at night when server load is at it's lowest although being new to Python I am not sure how efficient my code is or how I can improve it. The script runs fine however any advice would be beneficial.

import os
import subprocess

from subprocess import Popen

if os.getuid() != 0 :

    raise Exception("\n\nWARNING : YOU MUST RUN THIS APPLICATION AS SU\n\n")   # Check if user has sufficient privillages

Log = open('/var/www/ServerOverview.txt', 'a')                                 # Open log file for data entry


print('\n\n-----------------------------------------------------------------')
print('PROCESS STARTING - VIRUS DEFINITION UPDATE AND SCAN')
print('-----------------------------------------------------------------\n\n')

virusDef = Popen(['freshclam'])                                                # Update Virus Definitions
virusDef.wait()

if virusDef.poll() != 0 :

    Log.write('\n\n VIRUS DEFINITIONS UPDATE FAILED')                          # Output if Virus Definitions Update failed

else :

    Log.write('\n\n VIRUS DEFINITIONS UPDATE COMPLETE')                        # Output if Virus Definitions Update was completed

virusScan = Popen(['clamscan', '-r', '/'])                                     # Scan System
virusScan.wait()

if virusScan.poll() != 0 :

    Log.write('\n\n VIRUS SCAN FAILED')                                        # Output if Virus Scan failed

else :

    Log.write('\n\n VIRUS SCAN COMPLETE')                                      # Output if Virus Scan was completed

Log.close()                                                                    # Close Log File

Thank you!

[chriswelborn]

one thing that i do to trim code down a little is leave out the == or != when i know I will be dealing with small integers, like:

if proc.poll():
    # non-zero return from process equates to true
    print("failure")
else:
    # zero return from process equates to false
    print("success")

i've also been using the 'with' statement when dealing with files, maybe not as important on such a small script, but i like it anyway.

try:
    with open('/var/www/ServerOverview.txt', 'a') as Log:
        Log.write('some stuff')
        # file is closed at the end of 'with'
except IOError as exOS:
    # unable to open file, disk is full, file not found, etc.
    print("Unable to open file.")

[TrustyTony]

I would try to learn to use functions for clarity, import subprocess is superfluous and unused as only separaterly imported Popen is used. Try..except suggestion for opening suggested by chriswelborn is also good one.

Untested code to give the idea (those functions look nice to have, so let's also enable the import as module):

import os
from subprocess import Popen


# Error not warning as we raise exception from main
class PriviledgeError(Exception):
    pass

try:
    if os.getuid():
        raise PriviledgeError("Priviledge Error : YOU MUST RUN THIS APPLICATION AS SU")
except AttributeError:
    # Windows has not os.getuid()
    print 'Could not check priviledges, no getuid command'


def log_it(lf, task, message, words = ('COMPLETE', 'FAILED')):
    lf.write('\n\n' + message)
    lf.write(' ' + words[bool(task.poll())])

def do_and_wait(command, lf, message):
    task = Popen(command)                                                
    task.wait()
    log_it(lf, task, message)

def banner(text, n=60, char='-'):
    return '''

{line}
{message}
{line}

'''.format(line=n*char, message=text.upper().center(n))

if __name__ == '__main__':
    print(banner('PROCESS STARTING - VIRUS DEFINITION UPDATE AND SCAN'))

    with open('/var/www/ServerOverview.txt', 'a') as log:
        do_and_wait(['freshclam'], log, 'VIRUS UPDATE')
        do_and_wait(['clamscan', '-r', '/'], log, 'VIRUS SCAN')

[Octet]

Thanks for that, very thorough explanation and well explained!