5
\$\begingroup\$

I am self-taught when it comes to coding. I want to improve, so please give me advice if I have some bad habits or if there is in general a better way of doing this.

The idea of this code is to look up Excel files on a file server, check if they are flagged as "finalized" and if so, to create new records in a database with the key/ID from the excel file. Than, also copy further data from the Excel file to the record.

I think could have just created (aka inserted) a new record with all data, but to handle errors from wrong datatypes I decided to do it one-by-one.

There is also a filesDone.txt file that stores the full path of all files already copied or that are older and don't contain the "finalized" property.

I plan to run this as a service. It should run the loop every 5 minutes. The code will later run as a service and loop through every 5 minutes.

So simply put the code does:

  1. loads file names from filesDone.txt to a set
  2. loads file names from the filesystem/ a certain dir + subdirs to a set
  3. creates a list of the difference of the two sets
  4. iterates through the list (all remaining files)
    • looks, if they have been flagged as "finalized",
    • then for each row in the file:
    • creates a new record in the database
    • and adds values to given record (one by one)
  5. adds the processed file's name to the file of filenames.
  6. waits 300 seconds.

I am very new to coding so sorry for my dilettantish approach. At least it works so far.

#modules
import pandas as pd
import pyodbc as db
import xlwings as xw
import glob
import os
from datetime import datetime, date
from pathlib import Path
import time
import sys

#constants
tick_time_seconds = 300

line = ("################################################################################### \n")
pathTodo = "c:\\myXlFiles\\**\\*"
pathDone = ("c:\\Done\\")
pathError = ("c:\\Error\\")

sqlServer = "MyMachine\\MySQLServer"
sqlDriver = "{SQL Server}"
sqlDatabase="master"
sqlUID="SA"
sqlPWD="PWD" 

#functions
def get_list_of_files_by_extension(path:str, extension:str) -> list:
    """Recieves string patch and extension;
    gets list of files with corresponding extension in path;
    return list of file with full path."""
    fileList = glob.glob(path+extension, recursive=True)
    if not fileList:
        print("no found files")
    else:
        print("found files")
    return fileList

def write_error_to_log(description:str, errorString:str, optDetails=""):filesDone.txt
    """Recieves strings description errorstring and opt(ional)Details;
    writes the error with date and time in logfile with the name of current date;
    return nothing."""
    logFileName = str(date.today())+".txt"
    optDetails = optDetails+"\n"
    dateTimeNow = datetime.now()
    newError = "{0}\n{1}\n{2}{3}\n".format(line, str(dateTimeNow), optDetails, errorString)
    print(newError)
    with open(Path(pathError, logFileName), "a") as logFile:
        logFile.write(newError)

def sql_connector():
    """sql_connector: Recieves nothing;
    creates a connection to the sql server (conncetion details sould be constants);
    returns a connection."""
    return db.connect("DRIVER="+sqlDriver+"; \
                        SERVER="+sqlServer+"; \
                        DATABASE="+sqlDatabase+"; \
                        UID="+sqlUID+"; \
                        PWD="+sqlPWD+";")

def sql_update_builder(dbField:str, dbValue:str, dbKey:str) -> str:
    """ sql_update_builder: takes strings dbField, dbValue and dbKey;
    creates a sql syntax command with the purpose to update the value of the
    corresponding field with the corresponding key;
    returns a string with a sql command."""
    return "\
            UPDATE [tbl_Main] \
            SET ["+dbField+"]='"+dbValue+"' \
            WHERE ((([tbl_Main].MyKey)="+dbKey+"));"

def sql_insert_builder(dbKey: str) -> str:
    """ sql_insert_builder: takes strings  dbKey;
    creates a sql syntax command with the purpose to create a new record;
    returns a string with a sql command."""
    return "\
            INSERT INTO [tbl_Main] ([MyKey])\
            VALUES ("+dbKey+")"

def append_filename_to_fileNameFile(xlFilename):
    """recieves anywthing xlFilename;
    converts it to string  and writes the filename (full path) to a file;
    returns nothing."""
    with open(Path(pathDone, "filesDone.txt"), "a") as logFile:
        logFile.write(str(xlFilename)+"\n")
###################################################################################
###################################################################################
# main loop
while __name__ == "__main__":

    ###################################################################################
    """ 1. load filesDone.txt into set"""
    listDone = []
    print(line+"reading filesDone.txt in "+pathDone)
    try:
        with open(Path(pathDone, "filesDone.txt"), "r") as filesDoneFile:
            if filesDoneFile:
                print("file contains entries")
                for filePath in filesDoneFile:
                    filePath = filePath.replace("\n","")
                    listDone.append(Path(filePath))
    except Exception as err:
        errorDescription = "failed to read filesDone.txt from {0}".format(pathDone)
        write_error_to_log(description=errorDescription, errorString=str(err))
        continue
    else: setDone = set(listDone)
    ###################################################################################
    """ 2. load filenames of all .xlsm files into set"""
    print(line+"trying to get list of files in filesystem...")
    try: 
        listFileSystem = get_list_of_files_by_extension(path=pathTodo, extension=".xlsm")
    except Exception as err:
        errorDescription = "failed to read file system "
        write_error_to_log(description=errorDescription, errorString=str(err))
        continue
    else:
        listFiles = []
        for filename in listFileSystem:
            listFiles.append(Path(filename))
        setFiles = set(listFiles)
    ###################################################################################
    """ 3. create list of difference of setMatchingFiles and setDone"""
    print(line+"trying to compare done files and files in filesystem...")
    setDifference = setFiles.difference(setDone)
    ###################################################################################
    """ 4. iterate thru list of files """

    for filename in setDifference:
        """ 4.1 try: look if file is marked as "finalized=True";
        if the xlfile does not have sheet 7 (old ones)
        just add the xlfilename to the xlfilenameFile"""
        try:
            print("{0}trying to read finalized state ... of {1}".format(line, filename))
            filenameClean = str(filename).replace("\n","")
            xlFile = pd.ExcelFile(filenameClean)
        except Exception as err:
                errorDescription = "failed to read finalized-state from {0} to dataframe".format(filename)
                write_error_to_log(description=errorDescription, errorString=str(err))
                continue
        else:
            if "finalized" in xlFile.sheet_names:
                dataframe = xlFile.parse("finalized")
                print("finalized state ="+str(dataframe.iloc[0]["finalized"]))
                if dataframe.iloc[0]["finalized"] == False:
                    continue
            else: 
                append_filename_to_fileNameFile(filename) #add the xlfilename to the xlfilenameFile"
                continue
        ###################################################################################
        """ 4.2 try: read values to dataframe"""
        try:
            dataframe = pd.read_excel(Path(filename), sheet_name=4)
        except Exception as err:
                errorDescription = "Failed to read values from {0} to dataframe".format(filename)
                write_error_to_log(description=errorDescription, errorString=str(err))
                continue
        ###################################################################################
        """ 4.3 try: open connection to database"""
        print("{0}Trying to open connection to database {1} on {2}".format(line, sqlDatabase, sqlServer))
        try:
            sql_connection = sql_connector() #create connection to server
            stuff = sql_connection.cursor()
        except Exception as err:
            write_error_to_log(description="Failed to open connection:", errorString=str(err))
            continue
        ###################################################################################
        """ 4.4 try: write to database"""
        headers = list(dataframe) #copy header from dataframe to list; easier to iterate
        values = dataframe.values.tolist() #copy values from dataframe to list of lists [[row1][row2]...]; easier to iterate
        for row in range(len(values)): #iterate over lines
            dbKey = str(values[row][0]) #first col is key
            sqlCommandString = sql_insert_builder(dbKey=dbKey)
            """ 4.4.1 firts trying to create (aka insert) new record in db ..."""
            try: 
                print("{0}Trying insert new record with the id {1}".format(line, dbKey))
                stuff.execute(sqlCommandString)
                sql_connection.commit()
                print(sqlCommandString)
            except Exception as err:
                sql_log_string = " ".join(sqlCommandString.split()) #get rid of whitespace in sql command
                write_error_to_log(description="Failed to create new record in DB:", errorString=str(err), optDetails=sql_log_string)
            else: #if record was created add the values one by one:
                print("{0}Trying to add values to record with the ID {1}".format(line, dbKey))
            """ 4.4.2 ... than trying to add the values one by one"""
            for col in range(1, len(headers)): #skip col 0 (the key)
                dbField = str(headers[col]) #field in db is header in the excel sheet
                dbValue = str(values[row][col]) #get the corresponding value
                dbValue = (dbValue.replace("\"","")).replace("\'","") #getting rid of ' and " to prevent trouble with the sql command
                sqlCommandString = sql_update_builder(dbField, dbValue, dbKey) # calling fuction to create a sql update command string
                try: #try to commit the sql command
                    stuff.execute(sqlCommandString)
                    sql_connection.commit()
                    print(sqlCommandString)
                except Exception as err:
                    sql_log_string = " ".join(sqlCommandString.split()) #get rid of whitespace in sql command
                    write_error_to_log(description="Failed to add values in DB:", errorString=str(err), optDetails=sql_log_string)
        append_filename_to_fileNameFile(filename)

    print(line)
    # wait for a certain amount of time
    for i in range(tick_time_seconds, 0, -1):
                sys.stdout.write("\r" + str(i))
                sys.stdout.flush()
                time.sleep(1)
                sys.stdout.flush()
    print(line)
    #break # this is for debuggung
\$\endgroup\$
  • 1
    \$\begingroup\$ There are more sections numbered 4.2 than with any other number. I wasn't fond of line numbering when BASIC interpreters looked easier to implement that way. \$\endgroup\$ – greybeard Jan 4 at 0:35
  • \$\begingroup\$ I am not sure what you are trying to tell me. I like to structure my code with these numbers. And sometimes I add a point in between and then the numbering is all over the place and I have 4.2s twice. But do you think in general that structuring the code with comments like this is a bad habit? \$\endgroup\$ – herrwolken Jan 4 at 14:53
  • \$\begingroup\$ Regrettably, my comment doesn't read 4.1 which occurs twice. I think structuring code with whitespace useful. Comments are indispensable to document what shall be achieved by some code - for by what mechanisms, read/debug code. Numbering going over the place to the point of having numerical labels appearing twice in succession is an indication the approach is problematic, and going in the direction of Dewey Decimal(adding decimal points and decimals) is not a solution. \$\endgroup\$ – greybeard Jan 4 at 15:48
3
\$\begingroup\$

Time constants

Rather than writing

tick_time_seconds = 300

you're better off using a more expressive built-in:

from datetime import timedelta
tick_time = timedelta(minutes=5)

Unnecessary parens

...around this:

line = ("################################################################################### \n")

besides, you're better off writing

line = 80*'#'

Windows paths

These:

pathTodo = "c:\\myXlFiles\\**\\*"
pathDone = ("c:\\Done\\")
pathError = ("c:\\Error\\")

have a few issues. First, declare them as raw strings (r'') so that you don't have to double up on backslashes. Also, none of those parens are necessary. Finally, the variable names here should probably be in all caps due to these things being global string constants.

Passwords

I sure hope that this:

sqlPWD="PWD" 

isn't what I think it is. Don't hard-code passwords in your code. This is a whole thing - you're best to google a reasonable way to have a wallet accessible from Python.

Spelling

Recieves = Receives

firts = first

Manual path concatenation

path+extension

is something you probably don't want to do yourself. Refer to Path.with_suffix.

Generators

        listFiles = []
        for filename in listFileSystem:
            listFiles.append(Path(filename))
        setFiles = set(listFiles)

should just be

files = {Path(filename) for filename in all_files}

Note that your variable names shouldn't include the type (i.e. list), just a useful description of what they actually hold.

f-strings

print("{0}trying to read finalized state ... of {1}".format(line, filename))

can be

print(line)
print(f'Trying to read finalized state of {filename}')

The line print should be separated for clarity, since it's a different line on the output.

Boolean comparison

if dataframe.iloc[0]["finalized"] == False:

should be

if not dataframe.iloc[0]["finalized"]:
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ In general, thank you a lot, this really helps! >> Note that your variable names shouldn't include the type (i.e. list), just a useful description of what they actually hold.<< is there a specific reason to not do this? Or is this just a convention? \$\endgroup\$ – herrwolken Jan 4 at 0:29
  • \$\begingroup\$ The reason is that there are other, better, ways to indicate type - such as declarative type hinting. \$\endgroup\$ – Reinderien Jan 4 at 2:36
  • \$\begingroup\$ I found PEP 526. This is great! Thanks for that advice. \$\endgroup\$ – herrwolken Jan 4 at 14:44

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.