I have just been informed that the following code written by me is extremely poor. I have absolutely no idea why. It is memory efficient, and looks clean to me. But still the feedback is very poor. I have no clue on why. If someone can put some comments, I will be highly grateful. I have to pass file name from command line to make it work - actually that is what they asked.
import re
import numpy as np
from collections import Counter
import sys
def parse_server_log(path_to_file):
#First check whether it's a legal file name
try:
f = open(path_to_file)
except:
print "\nInvalid file name and/or path. Please correct!\n"
return
# end of sanity check on file
# The following regexes would extract the concerned values from each line
# of the file.
method_regx = re.compile("(method=)+[A-Z]+[\s]+")
path_regx = re.compile("(path=)+[/\w.\"]+[\s]+")
dyno_regx = re.compile("(dyno=)+[\w.]+[\s]+")
connect_regx = re.compile("(connect=)+[\w.]+[\s]+")
service_regx = re.compile("(service=)+[\w.]+[\s]+")
# Target values for each urls
url1 = [0, [], []]
url2 = [0, [], []]
url3 = [0, [], []]
url4 = [0, [], []]
url5 = [0, [], []]
url6 = [0, [], []]
# url matcher regex for each url type
url1_regex = re.compile("(#)+(/api/users/)+[\d]+(/count_pending_messages)+(#)+")
url2_regex = re.compile("(#)+(/api/users/)+[\d]+(/get_messages)+(#)+")
url3_regex = re.compile("(#)+(/api/users/)+[\d]+(/get_friends_progress)+(#)+")
url4_regex = re.compile("(#)+(/api/users/)+[\d]+(/get_friends_score)+(#)+")
url5_6_regex = re.compile("(#)+(/api/users/)+[\d]+(#)+")
with open(path_to_file) as lines:
for my_data in lines:
# Now lets separate out the method, path, dyno, connect and service times
k = method_regx.search(my_data)
try:
line_method = k.group(0).split("=")[1].strip()
except:
line_method = ""
k = path_regx.search(my_data)
try:
# The hashes are added at the start and end to make sure the path
# is not a part of a string rather the entire string!
line_path = "#" + k.group(0).split("=")[1].strip() + "#"
except:
line_path = ""
k = dyno_regx.search(my_data)
try:
line_dyno = k.group(0).split("=")[1].strip()
except:
line_dyno = ""
k = connect_regx.search(my_data)
try:
line_connect_time = float(k.group(0).split("=")[1].split("ms")[0])
except:
line_connect_time = 0
k = service_regx.search(my_data)
try:
line_service_time = float(k.group(0).split("=")[1].split("ms")[0])
except:
line_service_time = 0
# End of getting all the data
# Now match up the URL and do this under sanity checker
if(len(line_method) > 0 and len(line_path) > 0):
url_denoter = 0
if url1_regex.match(line_path) is not None:
url_denoter = 1
elif url2_regex.match(line_path) is not None:
url_denoter = 2
elif url3_regex.match(line_path) is not None:
url_denoter = 3
elif url4_regex.match(line_path) is not None:
url_denoter = 4
elif url5_6_regex.match(line_path) is not None:
url_denoter = 5
# OK so now we have the url to which the current url matched
if(url_denoter==1 and line_method=="GET"):
"""
for GET /api/users/{user_id}/count_pending_messages
"""
url1[0] += 1
url1[1].append(line_dyno)
url1[2].append(line_connect_time + line_service_time)
elif(url_denoter==2 and line_method=="GET"):
"""
for GET /api/users/{user_id}/get_messages
"""
url2[0] += 1
url2[1].append(line_dyno)
url2[2].append(line_connect_time + line_service_time)
"""
Now print the results!
"""
# for GET /api/users/{user_id}/count_pending_messages
print "\n------GET /api/users/{user_id}/count_pending_messages----\n"
print "Number of times the url is called: ", url1[0]
if(url1[0]>0):
my_num_list = url1[2]
print "Average response time: ", round(np.average(my_num_list), 2), " in ms."
print "Median response time: ", round(np.median(my_num_list), 2), " in ms."
print "Mode of response time: ", round(Counter(my_num_list).most_common(1)[0][0], 2), " in ms."
counts = [(x, url1[1].count(x)) for x in set(url1[1])]
swap = 0
tar_dyno = ""
for count in counts:
if(count[1]> swap):
swap = count[1]
tar_dyno = count[0]
print "Dyno that responded the most: ", tar_dyno
else:
print "Sorry no parameters can be calculated as the url has not been accessed!"
print "\n------GET /api/users/{user_id}/get_messages----\n"
print "Number of times the url is called: ", url2[0]
if(url2[0]>0):
my_num_list = url2[2]
print "Average response time: ", round(np.average(my_num_list), 2), " in ms."
print "Median response time: ", round(np.median(my_num_list), 2), " in ms."
print "Mode of response time: ", round(Counter(my_num_list).most_common(1)[0][0], 2), " in ms."
counts = [(x, url2[1].count(x)) for x in set(url2[1])]
swap = 0
tar_dyno = ""
for count in counts:
if(count[1]> swap):
swap = count[1]
tar_dyno = count[0]
print "Dyno that responded the most: ", tar_dyno
else:
print "Sorry no parameters can be calculated as the url has not beenaccessed!"
print "\n------GET /api/users/{user_id}/get_friends_progress----\n"
print "Number of times the url is called: ", url3[0]
if(__name__=="__main__"):
parse_server_log(sys.argv[1])
(#)+(/api/users/)+[\d]+(#)+
indicates that each group can be present several times? I don't think+
does what you think it does. Also, later you are not using the fact that you have groups. \$\endgroup\$except
statements should have explicit exception types. E.g., for catching non-existent filesexcept IOError:
. \$\endgroup\$