My script looks for handshake files in an input folder to determine whether certain processes have completed. If it finds them, it will update the database with the current time stamp.
Things I am wondering:
- My
update_db
function takes 5 parameters. I have heard that in that case there is probably a better way to do it. Would this be a good candidate for creating aclass
and using an OO approach? - I am eventually going to be using a datetime to compare against the handshake timestamps, would it be difficult to somehow pull the file dates off our Linux server instead of creating them in the script?
- Finally, any inconsistencies, ways I can simplify/improve my code. I am a new programmer of only 6 months, and love to learn:)
Here's the code:
def main():
# Get current time stamp
now = datetime.now()
formatted_dt = '{0}/{1}/{2}-{3}:{4}:{5}'.format(now.month,
now.day,
now.year,
now.hour,
now.minute,
now.second)
# Connect to db
sqlite_file = '/bcs/lgnt/clientapp/csvbill/database/csv.db'
csv_connection = sqlite3.connect(sqlite_file)
c = csv_connection.cursor()
file_type = ['auditv2', 'handshake']
# Check for files that don't have first hshake
c.execute('SELECT pivot_id FROM aud_qty WHERE hshake1 is NULL')
piv_id_list = [str(x[0]) for x in c.fetchall()]
missing_list = update_db('CSV1000', file_type[0], piv_id_list, c, formatted_dt)
# hshake1_miss = check_hshake('CSV1000', file_type[0], piv_id_list)
# print(hshake1_miss)
csv_connection.commit()
csv_connection.close()
def update_db(proj_id, audit_type, piv_id_list, db_cursor, time_stamp):
# chdir('/prod/bcs/lgnp/input')
# prod_list = listdir('/prod/bcs/lgnp/input/')
# for testing
chdir('/bcs/lgnt/input')
prod_list = listdir('/bcs/lgnt/input/')
hshake_list = []
for i, e in enumerate(prod_list):
prod_list[i] = prod_list[i].split('_') # Field 6 is auditv2, Field 7 is the Pivot ID
# Get only the Pivot ID
for i, e in enumerate(prod_list):
try:
if prod_list[i][0] == proj_id and prod_list[i][6] == audit_type:
hshake_list.append(prod_list[i][7])
except IndexError:
pass
# pdb.set_trace()
for i, e in enumerate(hshake_list):
if hshake_list[i] in piv_id_list:
print(hshake_list[i])
# pdb.set_trace()
db_cursor.execute('UPDATE aud_qty SET hshake1 = ? WHERE pivot_id = ?',
(time_stamp, hshake_list[i]))
piv_id_list.remove(hshake_list[i])
print(piv_id_list)
return piv_id_list
2 Answers 2
Working with resources
Whenever possible,
anything that must be closed after using,
in this example the database connection,
it's good to wrap it in a with ... as
, like this:
with sqlite3.connect(sqlite_file) as conn:
cursor = conn.cursor()
# ...
This way, conn
will be automatically closed when exiting the block,
so it's impossible to forget to close it, therefore more robust.
Using enumerate
enumarate
is really handy, but you're not using it to its full potential:
for i, e in enumerate(prod_list): prod_list[i] = prod_list[i].split('_')
Do you see the unused variable e
? Since you have it, why not use it:
for index, item in enumerate(prod_list):
prod_list[index] = item.split('_')
Naming
The posted code is filled with poor names, for example:
- the
main
function - the
c
cursor variable ->cursor
- the
i
loop variable for index ->index
- the
e
loop variable for an element ->item
At the minimum, try to avoid single-letter names.
mm/dd/yyyy format
If at all possible, I'd recommend to use any other date format.
Comprehensive map of all countries in the world that use the MMDDYYYY format:
Python actually has a date formatting function in datetime that makes it easier to do this than needing the usual string format method
now = datetime.now()
formatted_dt = '{0}/{1}/{2}-{3}:{4}:{5}'.format(now.month,
now.day,
now.year,
now.hour,
now.minute,
now.second)
Can just be
timestamp = datetime.now().strftime('%m/%d/%y-%H:%M:%S')
See the full table about the method here. You can do some really helpful stuff, like automatically getting am/pm and getting data in the user's locale format. Also you'll note I changed the name, I think naming it for its purpose is clearly and removes the need for the comment too.