Skip to main content
Code Review

Return to Answer

Fix logging imports
Source Link
import os
from datetime import datetime
from logging.handlerssys import TimedRotatingFileHandlerstdout
from sys import stdoutlogging
import mysql.connector
import MySQLdb
import base64
from ConfigParser import ConfigParser
from sendmail import send_mail
def parse_config(config_filename):
 """function that reads the config file"""
 config = ConfigParser()
 with open(config_filename) as config_file:
 config.readfp(config_file)
 return config
class FolderWatcher(object):
 def __init__(self, config_filename):
 """Read configuration file and store important data out of it"""
 config = parse_config(config_filename)
 app_name = config.get('application', 'app_name')
 self.logger = logging.getLogger(app_name)
 self.logger.setLevel(logging.DEBUG)
 formatter = logging.Formatter(
 '%(asctime)s - %(name)s - '
 '%(levelname)s: %(message)s')
 log_path = config.get('application', 'log_path')
 handler = logging.handlers.TimedRotatingFileHandler(
 os.path.join(log_path, app_name + '.log'),
 when="d", interval=1, backupCount=1)
 handler.setFormatter(formatter)
 self.logger.addHandler(handler)
 handler = logging.StreamHandler(sys.stdout)
 handler.setLevel(logging.DEBUG)
 self.logger.addHandler(handler)
 self.watched_folder = config.get('application','file_path')
 self.mysql_credentials = {
 'user': config.get('database','user'),
 ##decode password, remove D and L from the string
 'password': base64.decodestring(config.get('database','password')),
 'host': config.get('database','host'),
 'database': config.get('database','db'),
 'port': config.get('database','port'),
 }
 self.mail_sender = config.get('email', 'sender')
 # You’ll need to modify your config file to get this line to work
 self.mail_receivers = map(str.strip,
 config.get('email', 'receivers').split(';'))
 self.logger.info("Application {} starts running".format(app_name))
 self.logger.info("-"*80)
 def manage_exception(self, exception):
 self.logger.error(exception)
 send_mail(self.mail_sender, self.mail_receivers, exception)
 def insert_csv(self, file_path, filename):
 """insert csv to database"""
 try:
 cnx = mysql.connector.connect(**self.mysql_credentials)
 cursor = cnx.cursor()
 with open(file_path, 'rb') as f:
 data = f.read()
 curr_datetime = datetime.now().isoformat(' ')
 sql = ("INSERT INTO wyko_file(wyko_file,file_name,date_inserted)"
 " VALUES (%s,%s,%s)")
 cursor.execute(sql, (data, filename, curr_datetime))
 if cursor.rowcount != 0:
 self.logger.info("Transfer to database succesful!")
 cnx.commit()
 except (MySQLdb.OperationalError, MySQLdb.ProgrammingError) as e:
 self.manage_exception(e)
 finally:
 cursor.close()
 cnx.close()
 def watch(self):
 """main loop that watches the directory""" 
 try:
 for root, dirs, files in os.walk(self.watched_folder):
 for filename in files:
 file_path = os.path.join(root, filename)
 try:
 self.logger.info("Transferring file: "+filename)
 self.insert_csv(file_path, filename)
 except Exception as e:
 self.manage_exception(e)
 else:
 os.remove(file)
 self.logger.info(
 'File {} successfully removed'.format(file_path))
 except Exception as e:
 self.manage_exception(e)
if __name__ == "__main__":
 watcher = FolderWatcher(os.path.join('Config', 'config.ini'))
 watcher.watch()
import os
from datetime import datetime
from logging.handlers import TimedRotatingFileHandler
from sys import stdout
import mysql.connector
import MySQLdb
import base64
from ConfigParser import ConfigParser
from sendmail import send_mail
def parse_config(config_filename):
 """function that reads the config file"""
 config = ConfigParser()
 with open(config_filename) as config_file:
 config.readfp(config_file)
 return config
class FolderWatcher(object):
 def __init__(self, config_filename):
 """Read configuration file and store important data out of it"""
 config = parse_config(config_filename)
 app_name = config.get('application', 'app_name')
 self.logger = logging.getLogger(app_name)
 self.logger.setLevel(logging.DEBUG)
 formatter = logging.Formatter(
 '%(asctime)s - %(name)s - '
 '%(levelname)s: %(message)s')
 log_path = config.get('application', 'log_path')
 handler = logging.handlers.TimedRotatingFileHandler(
 os.path.join(log_path, app_name + '.log'),
 when="d", interval=1, backupCount=1)
 handler.setFormatter(formatter)
 self.logger.addHandler(handler)
 handler = logging.StreamHandler(sys.stdout)
 handler.setLevel(logging.DEBUG)
 self.logger.addHandler(handler)
 self.watched_folder = config.get('application','file_path')
 self.mysql_credentials = {
 'user': config.get('database','user'),
 ##decode password, remove D and L from the string
 'password': base64.decodestring(config.get('database','password')),
 'host': config.get('database','host'),
 'database': config.get('database','db'),
 'port': config.get('database','port'),
 }
 self.mail_sender = config.get('email', 'sender')
 # You’ll need to modify your config file to get this line to work
 self.mail_receivers = map(str.strip,
 config.get('email', 'receivers').split(';'))
 self.logger.info("Application {} starts running".format(app_name))
 self.logger.info("-"*80)
 def manage_exception(self, exception):
 self.logger.error(exception)
 send_mail(self.mail_sender, self.mail_receivers, exception)
 def insert_csv(self, file_path, filename):
 """insert csv to database"""
 try:
 cnx = mysql.connector.connect(**self.mysql_credentials)
 cursor = cnx.cursor()
 with open(file_path, 'rb') as f:
 data = f.read()
 curr_datetime = datetime.now().isoformat(' ')
 sql = ("INSERT INTO wyko_file(wyko_file,file_name,date_inserted)"
 " VALUES (%s,%s,%s)")
 cursor.execute(sql, (data, filename, curr_datetime))
 if cursor.rowcount != 0:
 self.logger.info("Transfer to database succesful!")
 cnx.commit()
 except (MySQLdb.OperationalError, MySQLdb.ProgrammingError) as e:
 self.manage_exception(e)
 finally:
 cursor.close()
 cnx.close()
 def watch(self):
 """main loop that watches the directory""" 
 try:
 for root, dirs, files in os.walk(self.watched_folder):
 for filename in files:
 file_path = os.path.join(root, filename)
 try:
 self.logger.info("Transferring file: "+filename)
 self.insert_csv(file_path, filename)
 except Exception as e:
 self.manage_exception(e)
 else:
 os.remove(file)
 self.logger.info(
 'File {} successfully removed'.format(file_path))
 except Exception as e:
 self.manage_exception(e)
if __name__ == "__main__":
 watcher = FolderWatcher(os.path.join('Config', 'config.ini'))
 watcher.watch()
import os
from datetime import datetime
from sys import stdout
import logging
import mysql.connector
import MySQLdb
import base64
from ConfigParser import ConfigParser
from sendmail import send_mail
def parse_config(config_filename):
 """function that reads the config file"""
 config = ConfigParser()
 with open(config_filename) as config_file:
 config.readfp(config_file)
 return config
class FolderWatcher(object):
 def __init__(self, config_filename):
 """Read configuration file and store important data out of it"""
 config = parse_config(config_filename)
 app_name = config.get('application', 'app_name')
 self.logger = logging.getLogger(app_name)
 self.logger.setLevel(logging.DEBUG)
 formatter = logging.Formatter(
 '%(asctime)s - %(name)s - '
 '%(levelname)s: %(message)s')
 log_path = config.get('application', 'log_path')
 handler = logging.handlers.TimedRotatingFileHandler(
 os.path.join(log_path, app_name + '.log'),
 when="d", interval=1, backupCount=1)
 handler.setFormatter(formatter)
 self.logger.addHandler(handler)
 handler = logging.StreamHandler(stdout)
 handler.setLevel(logging.DEBUG)
 self.logger.addHandler(handler)
 self.watched_folder = config.get('application','file_path')
 self.mysql_credentials = {
 'user': config.get('database','user'),
 ##decode password, remove D and L from the string
 'password': base64.decodestring(config.get('database','password')),
 'host': config.get('database','host'),
 'database': config.get('database','db'),
 'port': config.get('database','port'),
 }
 self.mail_sender = config.get('email', 'sender')
 # You’ll need to modify your config file to get this line to work
 self.mail_receivers = map(str.strip,
 config.get('email', 'receivers').split(';'))
 self.logger.info("Application {} starts running".format(app_name))
 self.logger.info("-"*80)
 def manage_exception(self, exception):
 self.logger.error(exception)
 send_mail(self.mail_sender, self.mail_receivers, exception)
 def insert_csv(self, file_path, filename):
 """insert csv to database"""
 try:
 cnx = mysql.connector.connect(**self.mysql_credentials)
 cursor = cnx.cursor()
 with open(file_path, 'rb') as f:
 data = f.read()
 curr_datetime = datetime.now().isoformat(' ')
 sql = ("INSERT INTO wyko_file(wyko_file,file_name,date_inserted)"
 " VALUES (%s,%s,%s)")
 cursor.execute(sql, (data, filename, curr_datetime))
 if cursor.rowcount != 0:
 self.logger.info("Transfer to database succesful!")
 cnx.commit()
 except (MySQLdb.OperationalError, MySQLdb.ProgrammingError) as e:
 self.manage_exception(e)
 finally:
 cursor.close()
 cnx.close()
 def watch(self):
 """main loop that watches the directory""" 
 try:
 for root, dirs, files in os.walk(self.watched_folder):
 for filename in files:
 file_path = os.path.join(root, filename)
 try:
 self.logger.info("Transferring file: "+filename)
 self.insert_csv(file_path, filename)
 except Exception as e:
 self.manage_exception(e)
 else:
 os.remove(file)
 self.logger.info(
 'File {} successfully removed'.format(file_path))
 except Exception as e:
 self.manage_exception(e)
if __name__ == "__main__":
 watcher = FolderWatcher(os.path.join('Config', 'config.ini'))
 watcher.watch()
Source Link

#Parsing configuration file

There are a few problems with your read_Config function:

  • ConfigParser objects are akin to dictionaries of dictionaries: there is no need to build dictionaries out of them, they already behave as such with a few extra functionnalities;

  • read_Config is called (and thus the file parsed) for each parameter you want to retrieve out of the file: this is a waste of resources, read the file once and return the parser object to extract parameters out of it;

  • read method of ConfigParser behave the same as open, it accepts relative path, no need to use getcwd for that;

  • read method of ConfigParser actually accept a list of filepaths (a single string is turned into a list of 1 element) and silently handle missing files (rationale in the docs); your code will fail if the file is not handled so I suggest using

     with open(<path>) as config_file:
     config.readfp(config_file)
    

    instead to get a more descriptive error if anything goes wrong.

#Too much "constants"

A lot of your constants are dependant of the result of your read_Config function.

At this point you may want to introduce a class in your design to improve readability and reusability. The various "constants" initialized out of read_Config will become the state (attributes) of your object.

#Exception handling

You made a good point of handling specific exceptions in the method reviewed in your previous question but forgot to apply the advice in your read_Config function.

Also you should prefer the new style of variable naming in except clause using the as keyword: e.g. except Exception as e. This is the only way to write it in Python 3 and yours is considered deprecated.

#Avoid repetitions

Almost all your except performs the same operation. For now it’s only two lines of code but someday you might want to improve them. Maybe that the send_mail function only uses str on your exception before sending the mail, leaving out the whole stacktrace with usefull informations and at some point you’ll want to improve that.

Use a function for that: any change will be applied anywhere for free.

#Bugs?

  • What is this write_log function in read_Config? You neither define it nor import it.
  • In main you use finally to remove files whether or not they got into the database. You may want to use else instead to remove only if the sql insert succeded.
  • You removed a while True from your last question, is it expected?

#Misc

  • Not sure which way is better but you can get your curr_daytimes using datetime.now().isoformat(' ') instead of using time;
  • Why are you using ast to parse the list of RECEIVERS? Can't you write it as a string of recipients separated by semi-colons and split it in your code?
  • Docstrings are "the string written the line right after a function declaration, if any" (not before).
  • smtplib, SMTP and PORT are never used.

#Proposed alternative

import os
from datetime import datetime
from logging.handlers import TimedRotatingFileHandler
from sys import stdout
import mysql.connector
import MySQLdb
import base64
from ConfigParser import ConfigParser
from sendmail import send_mail
def parse_config(config_filename):
 """function that reads the config file"""
 config = ConfigParser()
 with open(config_filename) as config_file:
 config.readfp(config_file)
 return config
class FolderWatcher(object):
 def __init__(self, config_filename):
 """Read configuration file and store important data out of it"""
 config = parse_config(config_filename)
 app_name = config.get('application', 'app_name')
 self.logger = logging.getLogger(app_name)
 self.logger.setLevel(logging.DEBUG)
 formatter = logging.Formatter(
 '%(asctime)s - %(name)s - '
 '%(levelname)s: %(message)s')
 log_path = config.get('application', 'log_path')
 handler = logging.handlers.TimedRotatingFileHandler(
 os.path.join(log_path, app_name + '.log'),
 when="d", interval=1, backupCount=1)
 handler.setFormatter(formatter)
 self.logger.addHandler(handler)
 handler = logging.StreamHandler(sys.stdout)
 handler.setLevel(logging.DEBUG)
 self.logger.addHandler(handler)
 self.watched_folder = config.get('application','file_path')
 self.mysql_credentials = {
 'user': config.get('database','user'),
 ##decode password, remove D and L from the string
 'password': base64.decodestring(config.get('database','password')),
 'host': config.get('database','host'),
 'database': config.get('database','db'),
 'port': config.get('database','port'),
 }
 self.mail_sender = config.get('email', 'sender')
 # You’ll need to modify your config file to get this line to work
 self.mail_receivers = map(str.strip,
 config.get('email', 'receivers').split(';'))
 self.logger.info("Application {} starts running".format(app_name))
 self.logger.info("-"*80)
 def manage_exception(self, exception):
 self.logger.error(exception)
 send_mail(self.mail_sender, self.mail_receivers, exception)
 def insert_csv(self, file_path, filename):
 """insert csv to database"""
 try:
 cnx = mysql.connector.connect(**self.mysql_credentials)
 cursor = cnx.cursor()
 with open(file_path, 'rb') as f:
 data = f.read()
 curr_datetime = datetime.now().isoformat(' ')
 sql = ("INSERT INTO wyko_file(wyko_file,file_name,date_inserted)"
 " VALUES (%s,%s,%s)")
 cursor.execute(sql, (data, filename, curr_datetime))
 if cursor.rowcount != 0:
 self.logger.info("Transfer to database succesful!")
 cnx.commit()
 except (MySQLdb.OperationalError, MySQLdb.ProgrammingError) as e:
 self.manage_exception(e)
 finally:
 cursor.close()
 cnx.close()
 def watch(self):
 """main loop that watches the directory""" 
 try:
 for root, dirs, files in os.walk(self.watched_folder):
 for filename in files:
 file_path = os.path.join(root, filename)
 try:
 self.logger.info("Transferring file: "+filename)
 self.insert_csv(file_path, filename)
 except Exception as e:
 self.manage_exception(e)
 else:
 os.remove(file)
 self.logger.info(
 'File {} successfully removed'.format(file_path))
 except Exception as e:
 self.manage_exception(e)
if __name__ == "__main__":
 watcher = FolderWatcher(os.path.join('Config', 'config.ini'))
 watcher.watch()
default

AltStyle によって変換されたページ (->オリジナル) /