My first program ever and it is written in python. I'm sure it's ful of buggs and flaws but it's tested and the code works... I hope someone could help me improve the code.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 | #!/usr/bin/python
# Check which version and uptime on the clients
import os
import sys
import ftplib
hosts = ["192.168.15.101",
"172.16.104.19",
"192.168.1.111",
"192.168.1.112",
"172.16.104.21",
"192.168.1.113",
"192.168.1.114",
"192.168.15.100",
"172.16.104.16",
"192.168.1.115",
"192.168.1.116",
"10.11.3.130"]
## Download /etc/borderlineversion and print the content of it
def getversion(ftp):
file = "borderlineversion"
ftp.cwd("/etc")
ftp.retrbinary("RETR " + file, open(file, "wb").write)
file = open("borderlineversion")
for line in file:
print ("Version: " +line.strip())
## Download /proc/uptime and than parse out the time from it and convert it to
## float and dived with 3600
def getuptime(ftp):
file = "uptime"
ftp.cwd("/proc")
ftp.retrbinary("RETR " + file, open(file, "wb").write)
file = open("uptime")
for line in file:
time = line.split()[0]
timefloat = float(time)
uptime = timefloat / 3600
print "Uptime: ", "%.2f" % uptime, "timmar\n"
## Loop through the list of hosts
for x in hosts:
print "Hosts: ", x
try:
ftp = ftplib.FTP(x)
ftp.login("****", "****")
except ftplib.error_perm, e:
print "Login failed %s" % e
sys.exit()
except socket.error, e:
print "Connection failed %s" % e
sys.exit()
getversion(ftp)
getuptime(ftp)
## clean up after all the work
ftp.close()
os.remove("borderlineversion")
os.remove("uptime")
|
Hi,
here's some suggestions:
file is a bif (until 3.0) ; don't use reserved name (bif, keyword ...) for your variables.
Keep the "executable/main" and the "library/reusable" parts separated.
The main loop (and the hosts list) should be in a " if __name__=='__main__': " block, or maybe in another module.
Don't mix the "processing" and the "interface" parts.
The get* functions are the processing part hence they shouldn't contain any print statment, as printing is part of the user interface.
Functions (and objects, and modules) are about "reusablity" and "maintainablity".
The get* functions are very much alike, you sould get what can be factorized out of those.
You could do a, say, def ftp_realines(ftp, file_name): kinda function which would return an iterable.
Thank you so much for taking you time helping me out Louis! Will try to rewrite the code in a more "right way" and I will consider all you inputs when doing it.