Kommentare MDL: == TICKET 1 == done, TEST THIS 20 # MDM Den Kram sollten wir wahrscheinlich aus der Konfigurationsdatei # MDM lesen rootURL = os.environ.get("GAVO_UPLOADBASE","http://dc.zah.uni-heidelberg.de/lightmeter/q/upload/custom") PROXY = None # GST: in VERSION = "0.1" ist das durch 94 class Config(object) gelöst, diese liest in der def __init__ die Variable source_path="~/.lightmeterrc" ein == TICKET 2 == TODO 65 # MDM: rather than? (was passiert, wenn moveUploaded falsch ist?) parser.add_option("-m", "--move-uploaded", action="store_true", dest="moveUploaded", help="Move files uploaded to a subdirectory uploaded?") # GST: alternativ könnten files einfach gelöscht werden, macht evtl. Sinn, wenn Platz gespart werden muss. Abfangen einbauen! == TICKET 3 == DONE 77 # MDM root_dir muss woandersher kommen. Es ist eine Pest, wenn # MDM die Programme davon abhängen, wo sie gestartet werden. Vorschlag: # MDM os.path.join(os.environ.get("HOME", "/root"), "lightmeter") root_dir=str(os.getcwd()), == TICKET 4 == DONE 84 # MDM parser.print_help, oder? parser.error('Error! Run with parameter -h for help') #GST: Done == TICKET 5 == TODO 90 #FIXME upload wirklich checken! opts.moveUploaded = True GST: wie? .methode upload muss success melden? (oder: 1. Vgl. mit logfile, 2. vgl. mit Dateien im uploaded folder ) == TICKET 6 == UNCLEAR 124 # MDM hier sollte das assert ersetzt werden durch irgendwas, das # MDM einen Fehler auslöst, der dann irgendwo geloggt wird. assert self.dev.write(self.endpointOutId, data, 0, 1000)==len(data) #GST: was bedeutet das? Google assert == TICKET 7 == UNCLEAR 175 # MDM current_log_name gefällt mir hier eigentlich nicht mehr, weils # MDM ja nicht eigentlich ein log ist (sondern die Daten). Na ja. self.current_log_name = os.path.join(self.root_dir, "current-measurement") self._open_log() def _open_log(self): self.current_log = open(self.current_log_name, "w") ... #GST: current_log_name durch current_filename ? == TICKET 8 == UNCLEAR 184 # FIXME: actually do some checking before writing ok. def log_line(self): output_line = "%s;%.1f;C;%d;%s;0;0;"%( time.strftime("%d.%m.%Y;%H:%M:%S", time.gmtime()), self.dev.getTemperature(), self.dev.getLight(), "ok \n") #GST: Was bringt dieses ok? Lightmeterwiki checken! == TICKET 9 == TODO 196 # MDM ich vermute, du willst hier lieber etwas machen wie # MDM queue_name = os.path.join(self.root_dir, "data", "uploading", # MDM opts.filename+time.strftime("%d%m%Y")+".csv") # MDM os.rename(...) -- auch wenn es wohl nicht mehr dramatisch wichtig # MDM ist, sorgt das für richtige Pfadtrenner in richtiger Zahl. def rotate(self): self.current_log.close() logname = opts.filename+time.strftime("%d%m%Y")+".csv" if VERBOSE: print "LOGNAME: ", logname Name_in_upload_queue = str("./data/Uploading/"+logname) os.rename(self.current_log_name, Name_in_upload_queue) if VERBOSE: print ""+self.current_log_name+" copied to "+Name_in_upload_queue self._open_log() == TICKET 10 == TODO 215 # MDM so viel logging find ich übertrieben für was doch sehr triviales. # MDM -- ich würde # if not os.path.isdir(newdir): # os.makedirs(newdir) # machen (das erzeugt im Zweifel gleich noch alles neue mit). def _mkdir(newdir): if os.path.isdir(newdir): if VERBOSE: print 'folder '+newdir+' exists' pass logging.info("folder "+newdir+" exists") else: os.mkdir(newdir) if VERBOSE: print 'creating folder %s' % repr(newdir) pass logging.info("creating folder "+str(repr(newdir))) == TICKET 11 == TODO 234 # MDM Hier wäre create_dir_hierarchy ein besserer Name, nein? # MDM Und: string wäre besser root_dir, die Pfade lieber über # MDM os.path.join zusammenbauen. def initialise(string): == TICKET 12 == DONE 399 # MDM das hier sollten wir auch aus einer richtigen Konfigurationsdatei # MDM lesen (Vorschlag: ~/.lightmeter) def _getCredentials(stationInfo): == TICKET 13 == UNCLEAR 529 # MDM Warum hier Beschränkung auf heute? Das Ding soll ruhig alles # MDM hochladen, was es findet. (drum aber auch fraglich, ob das # MDM Verschieben eigentlich abschaltbar sein soll...) todaysPat = getTodaysPattern() return [name for name in res if not todaysPat in name] #GST: eigentlich keine grosse Sache, der Grund ist wohl, das das file des heutigen Tages irgendwann mal "im Weg" war, inzwischen heisst die Datei ja current_log, sobald diese 24 Stunden fertig ist wird diese ja erst in die Datei des Tages umgeschrieben. == TICKET 14 == TODO 536 # MDM Vgl. _mkdir... def ensureDirectory(dirName): if not os.path.isdir(dirName): os.makedirs(dirName) == TICKET 15 == TODO 564 # MDM hier fände ich ein os.rename mit ordentlich konstruiertem # MDM Zielnamen eher günstiger (zumal das kracht, wenn du # MDM ein Überschreiben versuchst). shutil.move(fName,str("./data/localArchive/.")) == TICKET 16 == UNCLEAR 0 # MDM Ruhig mal den ganzen Kram mit upload_interval raushauen -- # MDM zur Not kann man das dann immer noch aus dem SVN ziehen # MDM Und die viele Ausgabe macht das Lesen der Funktion etwas # MDM anstrengend. Lieber weniger... (und print "" geht immer durch # MDM \n im string davor oder danach) #GST: ? == TICKET 17 == UNCLEAR 630 #MDM switch1 ist *immer* ein schlechter Variablenname. Generell ist # MDM hier zu viel bool'scher Zustand. In solchen Fällen ("event loop") # MDM lieber was wie: # MDM while True: # MDM if doomsday: # MDM break # MDM oder so. switch1=1 == TICKET 18 == TODO 641 # MDM das folgende wäre kürzer mit # MDM localDay, localHour, localMinute = time.localtime()[2:4] # MDM geschrieben, aber eigentlich willst du das lieber mit # MDM datetime machen. localtime=time.localtime(time.time()) localSecond=localtime[5] localMinute=localtime[4] localHour=localtime[3] localDay=localtime[2] == TICKET 19 == TODO 669 # MDM Diese ganze Logik würde ich lieber in einen Beobachter auslagern. # MDM Das würde dann so aussehen: # MDM upload_watcher = UploadTimer() # MDM while True: # MDM if upload_timer.expired(): # MDM jetzt upload machen # MDM upload_timer.rewind() # MDM ... usf # # MDM Der upload_timer.rewind() würde sich ausrechnen, wann der # MDM nächste Upload fällig ist und dann immer datetime.now() dagegen # MDM vergleichen -- dann sparst du dir die komplizierte Vergleicherei. if (str(localHour) == str(opts.upload_time)) and (trigger == 1): if VERBOSE: print "Upload already done at this time" else: trigger=0 == TICKET 20 == TO BE DISCUSSED, TESTED 706 # MDM Das sollte hier nicht nochmal aufgerufen werden opts,args = parse_command_line_parameters() #GST ich glaub schon, da sich der Dateiname ja zeitlich ändert muss der neu eingelesen werden bevor der upload startet. #GST natürlich ist das crappy gemacht, aber funzt ;) #GST wahrscheinlich soltlte ich da auch die Timer-Funktion von oben einbauen, #GST Problem: den Filenamen wollte ich ja in einem Configfile angebbar machen #GST weiter dann dynamisch ein Datum (das aktuelle) an diesen anhängen #GST in dieser Konstruktion habe ich die obige Lösung für die schnellste gehalten. == TICKET 21 == TODO/UNCLEAR 708 # MDM uploadFiles kann potenziell lang dauern oder hängen. Insofern # MDM solltest du das rausforken (vgl. doku von os.fork()) uploadFiles(opts, files) == TICKET 22 == UNCLEAR # GST: es gibt ja den Ordner local_archive, man könnte den Inhalt dieses local_archive mit dem uploaded-Ordner vergleichen um festzustellen, ob alle files bisher hochgeladen wurden. == TICKET 23 == UNCLEAR # GST: What does the conversion factorsforRange in GetLight really do?