4
\$\begingroup\$

I want to check some lines from a log file [check msyql replication slave status log] named "status.txt" (size is always about 3-4 KB):

Slave_IO_Running: Yes
Slave_SQL_Running: Yes

If any of these two line's value is 'No' then email is sent to alert@example.com. These two lines are unique and both occur only once.

#!/usr/bin/python
import smtplib
import os
import email.Utils
from email.mime.text import MIMEText
import sys
i=0
for line in open("status.txt"):
    if "Slave_IO_Running: Yes" in line:
        print "match1",line
        i=i+1
    else:
        pass       

for line2 in open("status.txt"):
    if "Slave_SQL_Running: Yes" in line2:
        i=i+1
    else:
        pass
print i

if i<2:
    msg=MIMEText("Mysql not running")
    fromaddr="root@DBausit.com"
    toaddr="alert@example.com"
    server = smtplib.SMTP('localhost')
    server.set_debuglevel(1)
    server.sendmail(fromaddr, [toaddr], msg.as_string())
    server.quit()
else:
    print "mysql running OK" 

Please review my code.

Contents of "status.txt" file:

    Relay_Log_Pos: 195294327
        Relay_Master_Log_File: web01-bin.000001
             Slave_IO_Running: Yes
            Slave_SQL_Running: Yes
              Replicate_Do_DB: 
          Replicate_Ignore_DB:
mysql,information_schema,cphulkd,eximstats,horde,leechprotect,logaholicDB_web01,performance_schema,roundcube,whmxfer
           Replicate_Do_Table: 
       Replicate_Ignore_Table:
\$\endgroup\$
4
\$\begingroup\$

The else-pass construct is unnecessary. The else clause is not required. Leaving it of has the same result as what you have now.


You loop over the same file twice. Instead you can check the line for both messages in the same loop.

for line in open("status.txt"):
    if "Slave_IO_Running: Yes" in line or "Slave_SQL_Running: Yes" in line2:
        print "match",line
        i=i+1

However, the above change points out a potential bug. We don't have any information about what the contents of status.txt are. If it is possible for either of those strings to appear more than once, the code as posted (and my above update) could find two matches without finding a result for IO and SQL. 1

sql_found = false
io_found = false
for line in open("status.txt"):
    if not io_found and "Slave_IO_Running: Yes" in line:
        print "match",line
        io_found = true
    if not sql_found and "Slave_SQL_Running: Yes" in line:
        print "match",line
        sql_found = true

if not sql_found or not io_found:
    # send email

If the status file is long, you can break out of the loop after both lines have been found. This would prevent unnecessary work when you already know the correct information.


i is not a good variable name. It does not convey any information about how it is being used. In addition, i is widely accepted as the variable name for indexing in a loop. In this code you are looping, but i is being used for something else. Changing the name will prevent any confusion.


You can use the with construct to ensure the file is closed when you are done with it.

with open(status) as f:
    for line in open("status.txt"):
        # operate on a line

This is a short running script, so it is not a huge deal, but it is a good practice to get into when you move to writing longer applications. While this is a short running


You never use sys, os or email.Utils. These imports can be removed.


1: If we extrapolate on the idea that the line might show up more than once, that could also mean that the file could contain a Yes line followed by a No line. If this is the case, this simple loop logic would no longer be sufficient.

| improve this answer | |
\$\endgroup\$

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.