Kleiner Daemon für Slideshow

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
Antworten
evilmonkey
User
Beiträge: 2
Registriert: Montag 9. Juni 2008, 21:31

Montag 9. Juni 2008, 21:37

Hallo, bin neu hier und stelle gleich mal ein kleines Tool vor welches ich zusammengeschustert habe.

Es sucht nach Bilddateien und wechselt in einem bestimmten Abstand dann das Hintergrundbild.

Code: Alles auswählen

#! /usr/bin/python

"""bgcd.py searches for jpg-files in a path and uses the tool feh to set it as background wallpaper.

Written by evilmonkey (there-is-an@evil-monkey-in-my-closet.com).

Usage: bgcd.py (start|stop)

You need the tool 'feh' to run bgcd.py

example: (for use with openbox):
	
	add the following line to your ~/.config/openbox/autostart.sh if bgcd.py is in your PATH
	(bgcd.py is in /usr/bin for example)
	
	bgcd.py start &
	
If you don't have a configfile (~./bgcd_config), bgcd.py automatically creates one for you. The first line is the path to your wallpapers, the second line the seconds until bgcd.py will switch to a new wallpaper. To stop bgcd, just run 'bgcd.py stop'

A logfile is created in /var/tmp/bgcd.log"""

__author__ = "there-is-an@evil-monkey-in-my-closet.com"
__copyright__ = "GPL"
__version__ = "0.7"

# Import libraries
import time
import os
import random
import sys

logfile="/var/tmp/bgcd.log"
configfile=os.getenv("HOME")+"/.bgcd_config"
support=[".jpg",".gif",".png",".pnm"]
example_config_text=os.getenv("HOME")+"/eee-wallpaper\n600"
pidfile="/tmp/bgcd.pid"

def log(text):
	"""log(text) writes 'text' to the configfile 'configfile'"""
	try:
		LOGFILE=open(logfile,'a')
		print >> LOGFILE,text
		LOGFILE.close()
	except:
		print "Error while opening file for log"

def remove_log():
	"""Removes the old log file"""
	try:
		os.remove(logfile)
	except:
		print "Could not remove old logfile"
	
def create_config_file():
	"""This function creates an example config file ~/.bgcd_config"""
	try:
		CONFIGFILE=open(configfile,'w')
		print >> CONFIGFILE,example_config_text
		CONFIGFILE.close()
	except:
		log ("Error writing config file. Exit now...")
		os._exit(0)
		
def start():
	os.chdir('/') 
	(sys.stdin,sys.stdout,sys.stderr) = [ open('/dev/null',m) for m in ('r','a','a')] 

	# Create a fork of the process to run as daemon
	pid = os.fork() 
	if pid: 
		# writting process ID to file
        	PIDFILE = open(pidfile,'w') 
        	print >>PIDFILE,pid 
        	PIDFILE.close() 
        	sys.exit(0) 
    	os.setsid() 
    	os.umask(0)
		
	# Remove old log-file
	remove_log()
	
	# Read config-file
	try:
		CONFIGFILE=open(configfile,'r')
	except:
		print "No config-file found, creating example config-file"
		create_config_file()
		CONFIGFILE=open(configfile,'r')
		
	path=CONFIGFILE.readline()
	tts=CONFIGFILE.readline()
	
	path=path.strip()
	tts=float(tts.strip())
	
	# Check if path exits
	if os.path.exists(path) == False:
		log (path + " not found! Exit...")
		os._exit(0)
	
	if path.endswith("/") == False:
		path=path+"/"
	dl=[]
		
	log ("Searching in:")
	log (path)
	log ("found images:")
	for dat in os.listdir(path):
		if os.path.isfile(path+dat):
			for suffix in support:
                		if dat.endswith(suffix):
					dl.append(path+dat)
					log (dat)

	number=len(dl)
	if number==0:
		log ("No files found! Exit...")
		os._exit(0)
	
	error=False
	old=-1
		
	while not error:
		try: 
			i=random.randrange(0, number)
			if i==old:
				i=i+1
				if i>=len(dl):
					i=0	
			old=i
			log ("switching to :")
			log (dl[i])
			os.system("feh --bg-center "+dl[i])
			time.sleep(tts)
		except:
			error=True
			log ("ERROR........")


def stop():
	try:
		PIDFILE = open(pidfile,'r')
		pid=PIDFILE.read()
		os.kill(int(pid.strip()),15)
		log("Terminated by bgcd.py stop")
	except:
		print "Could not stop process"
	

def usage():
	print "Usage: bgcd.py (start|stop)"
	
def main():
	
	if len(sys.argv)<2:
		sys.exitfunc=usage
		sys.exit()
	
	if sys.argv[1]=="start":
		start()
	else:
		if sys.argv[1]=="stop":
			stop()
		else:
			sys.exitfunc=usage
			sys.exit()
	

if __name__ == "__main__":
	main()
Ich habe das Script auf meinem eee-pc mit openbox laufen, funktioniert sehr gut :)

Verbesserungsvorschläge und Meinungen sind willkommen!
lunar

Dienstag 10. Juni 2008, 11:37

Naja... ;) Ich gehe das einfach mal von oben nach unten durch ;)

Auch in Docstrings solltest du die von PEP 8 vorgeschlagene maximale Zeilenlänge von 80 Zeichen einhalten. Scrollbalken im Code-Block sind halt hässlich ;)

Grundsätzlich solltest du Konstanten mit Großbuchstaben bezeichnen, damit man sie im Code auch erkennt. Das betrifft die fünf Namen, die du auf Modulebene bindest.

Pfade setzt man mit os.path.join zusammen. ``os.getenv`` ist zwar nicht falsch, aber imho ist der Zugriff auf ``os.environ`` eher üblich.

Für Logging gibt es das gleichnamige Modul in der Standardbibliothek, es besteht also kein Grund, Logging selbst zu implementieren.

Vor allem, weil dein Logging nicht sehr performant ist, da die Datei bei jedem Logeintrag neu geöffnet werden muss. Außerdem wird die Datei nicht richtig geschlossen, wenn in Zeile 42 eine Ausnahme auftritt. Das führt dazu, dass geöffnete Dateideskriptoren zurückbleiben. Dadurch besteht die theoretische Möglichkeit, dass der Kernel dein Skript killt, weil es bei langen Laufzeiten die Ressourcenlimits überschreitet.

In Zeile 44 verfällst du einer in letzter Zeit imho ziemlich häufig zu beobachtenden Unsitte: Ein except ohne Ausnahmetyp. Damit fängst du alles ab, auch Syntaxfehler, was sicherlich nicht in deinem Interesse ist. Wenn du IO-Ausnahmen behandeln willst, solltest du nur EnvironmentError abfangen. Ein einfaches except ist höchstens dann sinnvoll, wenn du den Traceback loggst, also beispielsweise ``logging.exception`` verwendest (ein weiterer Grund, dass ``logging`` zu verwenden). Generell solltest du ein ``except`` ohne Ausnahme aber meiden wie der Teufel das Weihwasser ;) Das gilt natürlich für jedes ``except`` in deinem Code.

Ob es Aufgabe eines Dämons ist, die Beispielkonfiguration zu erzeugen, sei mal dahingestellt. Eigentlich liefert man die Beispiel-Konfiguration mit dem Programm aus, so dass der Nutzer sie auch vor dem ersten Programmstart zur Verfügung hat.

Zeile 66 ist eher sinnlos. Damit öffnest du nur die Dateiobjekte neu, die Standardströme bleiben trotzdem mit dem Terminal verbunden, da die Dateideskriptoren 0, 1 und 2 nicht neu vergeben werden. Im Gegenteil, du erzeugst einfach drei neue Dateideskriptoren 3, 4 und 5, die auf /dev/null verweisen. Korrekterweise solltest du zuerst alle Dateideskriptoren schließen, um alle Verbindungen zur Umgebung des Vaterprozesses zu schließen. Danach solltest du 0, 1 und 2 mit ``os.open`` neu öffnen, um die Dateideskriptoren neu zu binden, so dass auch C-Code nicht mehr die alten Standardströme nutzen kann.

Im Cookbook gibt es ein Rezept zum Forken eines Dämons, dass du dir ansehen solltest.

Zum Parsen einer Konfigurationsdatei solltest du eines der verfügbaren Module wie ConfigParser oder ConfigObj nehmen, die gehen auch mit nicht existierenden Konfigurationsdateien korrekt um, da du Standardwerte zuweisen kannst. Dann sparst du dir die Krücke in Zeile 83 ff.

Außerdem schließen diese Module die Konfigurationsdatei wieder, so dass die benötigten Ressourcen wieder freigeben werden.

In Zeile 94 fehlt die die komplette Fehlerbehandlung. Wenn der Nutzer keine korrekte Fließkommazahl in der Konfiguration angibt, stürzt dein Programm stillschweigend ab. Wobei ich mir nicht mal sicher bin, ob der Traceback wirklich verloren geht. Immerhin ist Dateideskriptor 2, also der Standardfehlerstrom noch offen, wenn die Ausnahme also in C-Code abgefangen wird, könnte der Traceback sogar noch auf dem Terminal erscheinen.

Zeile 101 und 102 kannst du dir komplett sparen, dass ist nur ein eher schlechter Workaround, zu dem du gezwungen bist, weil du os.path.join nicht verwendest. Gleiches gilt für die Stringkonkatenation in Zeile 109 und 112.

In Zeile 112, 113 ist deine Einrückung imho kaputt.

Wenn du, wie in Zeile 116 überprüfen willst, ob eine Liste leer ist, nutze die Tatsache, dass leere Listen im bool'schen Kontext als ``False`` ausgewertet werden: Statt ``if len(dl) == 0:`` also ``if not dl:``.

Du wirst jetzt sagen, dass du die Länge eh in Zeile 125 nochmal brauchst, aber statt einen zufälligen Index in der Liste auszuwählen, kannst du auch gleich direkt das Element "ziehen": ``random.choice`` ist das Mittel der Wahl ;)

Bezüglich Zeile 133: Hast du das Programm schon mal mit Dateinamen, die Leerzeichen oder Anführungszeichen enthalten, ausprobiert? Du wirst feststellen, dass das nicht funktioniert. ``os.system`` nutzt die Shell zum Starten des Unterprozesses, ergo musst du korrekt quoten. Am besten nimmst du aber gleich das ``subprocess``-Modul.

Zeile 156, 157 ist sehr merkwürdig. Warum nur machst du das so obskur? Du könntest ``usage`` auch gleich direkt aufrufen. Oder das Feature nutzen, dass ``sys.exit`` auch einen String als Argument akzeptiert.

Bezüglich Zeile 161: ``elif`` existiert.

my 2 cents
lunar

Wenn du schon in der stop-Funktion SIGTERM zum Töten des Prozesses nutzt, dann solltest du das auch richtig abfangen, um den Prozess sauber zu beenden.
evilmonkey
User
Beiträge: 2
Registriert: Montag 9. Juni 2008, 21:31

Dienstag 10. Juni 2008, 19:07

lunar hat geschrieben:Naja... ;) Ich gehe das einfach mal von oben nach unten durch ;)
Schön das sich jemand die Mühe macht ;)
lunar hat geschrieben: Auch in Docstrings solltest du die von PEP 8 vorgeschlagene maximale Zeilenlänge von 80 Zeichen einhalten. Scrollbalken im Code-Block sind halt hässlich ;)
Kommt davon wenn man vom Editor einfach den Text in den Browser kopiert :)
lunar hat geschrieben: Grundsätzlich solltest du Konstanten mit Großbuchstaben bezeichnen, damit man sie im Code auch erkennt. Das betrifft die fünf Namen, die du auf Modulebene bindest.
Da hast du natürlich recht, aber die globalen Variablen wollte ich sowieso noch loswerden
lunar hat geschrieben: Pfade setzt man mit os.path.join zusammen. ``os.getenv`` ist zwar nicht falsch, aber imho ist der Zugriff auf ``os.environ`` eher üblich.
Danke, guter Tipp, werde ich ins Zukunft benutzen :)
lunar hat geschrieben: Für Logging gibt es das gleichnamige Modul in der Standardbibliothek, es besteht also kein Grund, Logging selbst zu implementieren.

Vor allem, weil dein Logging nicht sehr performant ist, da die Datei bei jedem Logeintrag neu geöffnet werden muss. Außerdem wird die Datei nicht richtig geschlossen, wenn in Zeile 42 eine Ausnahme auftritt. Das führt dazu, dass geöffnete Dateideskriptoren zurückbleiben. Dadurch besteht die theoretische Möglichkeit, dass der Kernel dein Skript killt, weil es bei langen Laufzeiten die Ressourcenlimits überschreitet.
Mag ja stimmen, aber bei einem Skript was alle 10min einen Befehl ausführt ist die Performance ja nicht unbedingt ein Hauptkriterium. Ich hab hier einfach den leichten Weg der Mikrooptimierung vorgezogen ;)
lunar hat geschrieben: In Zeile 44 verfällst du einer in letzter Zeit imho ziemlich häufig zu beobachtenden Unsitte: Ein except ohne Ausnahmetyp. Damit fängst du alles ab, auch Syntaxfehler, was sicherlich nicht in deinem Interesse ist. Wenn du IO-Ausnahmen behandeln willst, solltest du nur EnvironmentError abfangen. Ein einfaches except ist höchstens dann sinnvoll, wenn du den Traceback loggst, also beispielsweise ``logging.exception`` verwendest (ein weiterer Grund, dass ``logging`` zu verwenden). Generell solltest du ein ``except`` ohne Ausnahme aber meiden wie der Teufel das Weihwasser ;) Das gilt natürlich für jedes ``except`` in deinem Code.
Das ist wohl einfach nur Bequemlichkeit. Nicht schön, aber naja.....
Wieder so eine "Quick&Dirty"-Unsitte...
lunar hat geschrieben: Ob es Aufgabe eines Dämons ist, die Beispielkonfiguration zu erzeugen, sei mal dahingestellt. Eigentlich liefert man die Beispiel-Konfiguration mit dem Programm aus, so dass der Nutzer sie auch vor dem ersten Programmstart zur Verfügung hat.
Tja, von den ersten 10 Leuten die mein Script anfangs benutzten waren 6 Stück leider nicht in der Lage, "echo $home/dir > .bgcd_config && echo 300 >> .bgcd_config " einzugeben. Natürlich ist das nicht die Aufgabe eines Daemons, aber es schadet ja auch nicht, oder?
lunar hat geschrieben: Zeile 66 ist eher sinnlos. Damit öffnest du nur die Dateiobjekte neu, die Standardströme bleiben trotzdem mit dem Terminal verbunden, da die Dateideskriptoren 0, 1 und 2 nicht neu vergeben werden. Im Gegenteil, du erzeugst einfach drei neue Dateideskriptoren 3, 4 und 5, die auf /dev/null verweisen. Korrekterweise solltest du zuerst alle Dateideskriptoren schließen, um alle Verbindungen zur Umgebung des Vaterprozesses zu schließen. Danach solltest du 0, 1 und 2 mit ``os.open`` neu öffnen, um die Dateideskriptoren neu zu binden, so dass auch C-Code nicht mehr die alten Standardströme nutzen kann.

Im Cookbook gibt es ein Rezept zum Forken eines Dämons, dass du dir ansehen solltest.
Habe ich getan und festgestellt, dass das wohl auch nicht der Weißheit letzter Schluss ist. Ist es wirklich nötig zweimal zu 'forken'? Andere Tutorials/HowTo's etc. kommen auch prima mit einem Fork aus.
lunar hat geschrieben: Zum Parsen einer Konfigurationsdatei solltest du eines der verfügbaren Module wie ConfigParser oder ConfigObj nehmen, die gehen auch mit nicht existierenden Konfigurationsdateien korrekt um, da du Standardwerte zuweisen kannst. Dann sparst du dir die Krücke in Zeile 83 ff.

Außerdem schließen diese Module die Konfigurationsdatei wieder, so dass die benötigten Ressourcen wieder freigeben werden.
Danke, werde ich mir mal anschauen
Ich beschäftige mich erst seit einer Woche mit Python, da kenne ich bei weitem noch nicht alles. Deswegen finde ich es gut auf sowas hingewiesen zu werden :)
lunar hat geschrieben: In Zeile 94 fehlt die die komplette Fehlerbehandlung. Wenn der Nutzer keine korrekte Fließkommazahl in der Konfiguration angibt, stürzt dein Programm stillschweigend ab. Wobei ich mir nicht mal sicher bin, ob der Traceback wirklich verloren geht. Immerhin ist Dateideskriptor 2, also der Standardfehlerstrom noch offen, wenn die Ausnahme also in C-Code abgefangen wird, könnte der Traceback sogar noch auf dem Terminal erscheinen.
Stimmt, habe das Python-Typecasting wohl überschätzt.
lunar hat geschrieben: Zeile 101 und 102 kannst du dir komplett sparen, dass ist nur ein eher schlechter Workaround, zu dem du gezwungen bist, weil du os.path.join nicht verwendest. Gleiches gilt für die Stringkonkatenation in Zeile 109 und 112.
Ein Workaround ja, aber warum soll der schlecht sein? :)
Und warum os.path.join(path,dat) verwenden, wenn auch path+dat das gleiche Ergebniss liefert, wo ich doch genau weiß, dass path+dat einen validen Pfad ergibt?
lunar hat geschrieben: In Zeile 112, 113 ist deine Einrückung imho kaputt.
Da kann man sich die fehlenden Tabulatoren einfach hindenken ;)
lunar hat geschrieben: Wenn du, wie in Zeile 116 überprüfen willst, ob eine Liste leer ist, nutze die Tatsache, dass leere Listen im bool'schen Kontext als ``False`` ausgewertet werden: Statt ``if len(dl) == 0:`` also ``if not dl:``.
Naja, das ist wohl eher Geschmackssache, genauso wie manche lieber i=i+1 statt i+=1 schreiben.
lunar hat geschrieben: Du wirst jetzt sagen, dass du die Länge eh in Zeile 125 nochmal brauchst, aber statt einen zufälligen Index in der Liste auszuwählen, kannst du auch gleich direkt das Element "ziehen": ``random.choice`` ist das Mittel der Wahl ;)
Guter Tipp, werd' ich mir merken
lunar hat geschrieben: Bezüglich Zeile 133: Hast du das Programm schon mal mit Dateinamen, die Leerzeichen oder Anführungszeichen enthalten, ausprobiert? Du wirst feststellen, dass das nicht funktioniert. ``os.system`` nutzt die Shell zum Starten des Unterprozesses, ergo musst du korrekt quoten. Am besten nimmst du aber gleich das ``subprocess``-Modul.
Leerzeichen oder Anführungszeichen in Dateinamen?
Wer macht den sowas?
Solche Unsitten gibt's bei mir nicht.....
lunar hat geschrieben: Zeile 156, 157 ist sehr merkwürdig. Warum nur machst du das so obskur? Du könntest ``usage`` auch gleich direkt aufrufen. Oder das Feature nutzen, dass ``sys.exit`` auch einen String als Argument akzeptiert.
Wollte ich auch schon längst geändert haben :roll:
lunar hat geschrieben: Bezüglich Zeile 161: ``elif`` existiert.
Naja, bei 2 if-Zweigen kann man das doch verkraften ;)
lunar hat geschrieben: Wenn du schon in der stop-Funktion SIGTERM zum Töten des Prozesses nutzt, dann solltest du das auch richtig abfangen, um den Prozess sauber zu beenden.
Und wie sollte das dann genau aussehen?
Weiß nicht warum das nicht reichen sollte, aber ich lasse mich da gerne aufklären...

Danke für Deine Kommentare, werde mir einige Anregungen mal genauer Anschauen. Du weißt ja, wenn man nicht fragt, erfährt man nichts. Deshalb ist es wichtig seinen Mist mal irgendwo zu posten ;)

Ach ja, hier mal mein ursprüngliches 3 Minuten Script, bevor ich daraus ein Daemon hacken wollte:

Code: Alles auswählen

import time
import os
import random
import sys

dl=[]
old=-1

path=sys.argv[1]
tts=float(sys.argv[2])

for dat in os.listdir(path):
    if os.path.isfile(path+dat):
        if dat.endswith(".jpg"):
            dl.append(path+dat)

while True:
    i=random.randrange(0, len(dl))
    while i==old:
        i=random.randrange(0, len(dl))     
    old=i
    os.system("feh --bg-center " +dl[i])    
    time.sleep(tts)
Nicht weltklasse, aber schnell hergestellt und funktioniert ;)
Leonidas
Administrator
Beiträge: 16024
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Dienstag 10. Juni 2008, 22:41

evilmonkey hat geschrieben:
lunar hat geschrieben: Ob es Aufgabe eines Dämons ist, die Beispielkonfiguration zu erzeugen, sei mal dahingestellt. Eigentlich liefert man die Beispiel-Konfiguration mit dem Programm aus, so dass der Nutzer sie auch vor dem ersten Programmstart zur Verfügung hat.
Tja, von den ersten 10 Leuten die mein Script anfangs benutzten waren 6 Stück leider nicht in der Lage, "echo $home/dir > .bgcd_config && echo 300 >> .bgcd_config " einzugeben. Natürlich ist das nicht die Aufgabe eines Daemons, aber es schadet ja auch nicht, oder?
Lunar meinte, dass du eine ``DOT.bgcd_config.sample`` mitlieferst. Diese umzubenennen sollte kein Problem sein. Wobei ich es seltsam finde, Daemons mit "unsichtbaren" Configdateien zu betreiben.
evilmonkey hat geschrieben:Ist es wirklich nötig zweimal zu 'forken'? Andere Tutorials/HowTo's etc. kommen auch prima mit einem Fork aus.
Ja. Ich meine mich erinnern zu können, dass es auf einigen Unices nötig ist, sonst bleiben da Deskriptoren oder so offen. Ich weiß es nicht mehr genau, aber mit zweimal forken bist du definitiv auf der sicheren Seite.
evilmonkey hat geschrieben:Ein Workaround ja, aber warum soll der schlecht sein? :)
Und warum os.path.join(path,dat) verwenden, wenn auch path+dat das gleiche Ergebniss liefert, wo ich doch genau weiß, dass path+dat einen validen Pfad ergibt?
Nicht jedes Betriebssystem akzeptiert die gleichen Pfadseperatoren und kommt mit mehreren Pfadseperatoren hintereinander zurecht. Man sollte sich gar nicht erst angewöhnen platformspezifisch zu programmieren, wenn es nicht auf triviale weise besser geht.
evilmonkey hat geschrieben:
lunar hat geschrieben: In Zeile 112, 113 ist deine Einrückung imho kaputt.
Da kann man sich die fehlenden Tabulatoren einfach hindenken ;)
Also mein Interpreter tuts nicht, deiner etwa? :P
evilmonkey hat geschrieben:
lunar hat geschrieben: Wenn du, wie in Zeile 116 überprüfen willst, ob eine Liste leer ist, nutze die Tatsache, dass leere Listen im bool'schen Kontext als ``False`` ausgewertet werden: Statt ``if len(dl) == 0:`` also ``if not dl:``.
Naja, das ist wohl eher Geschmackssache, genauso wie manche lieber i=i+1 statt i+=1 schreiben.
Ist aber performanter und klarer. Ein Wer ist False wenn nichts drin ist. So spart man sich einen Funktionsaufruf und der Code ist so oder so kürzer. ``i+=1`` ist übrigens auch schneller, weil ``i`` nur einmal im Namespace gefunden werden muss statt zweimal.
evilmonkey hat geschrieben:Leerzeichen oder Anführungszeichen in Dateinamen?
Wer macht den sowas?
Solche Unsitten gibt's bei mir nicht.....
Die Einstellung finde ich immer toll, wenn Umlaute falsch dargestellt werden, weil sich die (amerikanischen) Autoren genau das gedacht haben. Ich kann ja nichts dafür, dass die Deutschen Umlaute im Namen haben, aber es ist nun mal so. Ein Programm was damit nicht zurechtkommt sollte im Jahr 2008 wirklich nicht mehr sein. Klar, passieren kanns, aber ignorieren sollte mans nicht, wenns so einfach zu beheben ist wie in Python.

Soll natürlich nicht bös gemeint sein, aber ich muss sagen, dass ich Lunar so ziemlich in jedem Punkt zustimme.
My god, it's full of CARs! | Leonidasvoice vs Modvoice
Benutzeravatar
birkenfeld
Python-Forum Veteran
Beiträge: 1603
Registriert: Montag 20. März 2006, 15:29
Wohnort: Die aufstrebende Universitätsstadt bei München

Mittwoch 11. Juni 2008, 06:54

evilmonkey hat geschrieben:
lunar hat geschrieben: Auch in Docstrings solltest du die von PEP 8 vorgeschlagene maximale Zeilenlänge von 80 Zeichen einhalten. Scrollbalken im Code-Block sind halt hässlich ;)
Kommt davon wenn man vom Editor einfach den Text in den Browser kopiert :)
Das meinte Lunar eher anders... auch im Sourcefile möchte man keine langen Zeilen haben.
lunar hat geschrieben: Zeile 101 und 102 kannst du dir komplett sparen, dass ist nur ein eher schlechter Workaround, zu dem du gezwungen bist, weil du os.path.join nicht verwendest. Gleiches gilt für die Stringkonkatenation in Zeile 109 und 112.
Ein Workaround ja, aber warum soll der schlecht sein? :)
Und warum os.path.join(path,dat) verwenden, wenn auch path+dat das gleiche Ergebniss liefert, wo ich doch genau weiß, dass path+dat einen validen Pfad ergibt?
Schauen wir mal...
[/code=py]
if path.endswith("/") == False:
path=path+"/"
...
if os.path.isfile(path+dat):
[/code]
gegen

Code: Alles auswählen

        if os.path.isfile(os.path.join(path, dat)):
Die erste Möglichkeit ist nicht nur zwei Zeilen länger, sondern läuft auch nur auf Plattformen mit Slash als Pfadseparator. Deswegen ist sie schlechter.
Dann lieber noch Vim 7 als Windows 7.

http://pythonic.pocoo.org/
EyDu
User
Beiträge: 4871
Registriert: Donnerstag 20. Juli 2006, 23:06
Wohnort: Berlin

Mittwoch 11. Juni 2008, 07:21

Und die

Code: Alles auswählen

if expr == False:
    ...
Teile kannst du auch als

Code: Alles auswählen

if not expr:
    ...
schreiben.
lunar

Mittwoch 11. Juni 2008, 11:33

evilmonkey hat geschrieben:Mag ja stimmen, aber bei einem Skript was alle 10min einen Befehl ausführt ist die Performance ja nicht unbedingt ein Hauptkriterium. Ich hab hier einfach den leichten Weg der Mikrooptimierung vorgezogen ;)
Die Verwendung des logging-Moduls resultiert in kürzerem Code ;)
Das ist wohl einfach nur Bequemlichkeit. Nicht schön, aber naja.....
Naja? Wenn du auf erschwertes Debugging stehst, dann kannst du ja weiter ``except:`` nutzen. ;) Vernünftige Programmierer fangen nur die Ausnahmen ab, an denen sie interessiert sind, der Rest geht als Traceback auf die Konsole oder ins Logging ;)
evilmonkey hat geschrieben:
lunar hat geschrieben:Im Cookbook gibt es ein Rezept zum Forken eines Dämons, dass du dir ansehen solltest.
Habe ich getan und festgestellt, dass das wohl auch nicht der Weißheit letzter Schluss ist. Ist es wirklich nötig zweimal zu 'forken'? Andere Tutorials/HowTo's etc. kommen auch prima mit einem Fork aus.
Der doppelte Fork ist nur dann nötig, wenn der Parent weiterläuft. Dann nämlich übernimmt init nicht die Vaterschaft des Kindprozesses, sondern sieht den Vater in der Pflicht ``SIGCHLD`` abzufangen und ``os.wait`` aufzurufen. Wenn der Vater das nicht tut, gibt es Zombies ;) In deinem Fall ist ein doppelter Fork nicht nötig, weil der Vaterprozess sich bei dir sofort beendet. Im Zweifelsfall ist dein doppelter Fork allerdings die sichere Lösung.

Mir ging es allerdings nicht um den Fork, sondern um das Neuzuweisen der Standardströme. Dein Code weist nur ``sys.stdout``, ``sys.stdin`` und ``sys.stderr`` neu zu und **nicht** die Standardeingabe (fd 0), die Standardausgabe (fd 1) und den Standardfehlerstrom (fd 2). Diese Dateideskriptoren verweisen noch immer auf das kontrollierende Terminal, dass der Prozess eigentlich gar nicht mehr hat ;) Du musst ``os.open`` nutzen, wie in dem Rezept illustriert.
Ein Workaround ja, aber warum soll der schlecht sein? :)
Und warum os.path.join(path,dat) verwenden, wenn auch path+dat das gleiche Ergebniss liefert, wo ich doch genau weiß, dass path+dat einen validen Pfad ergibt?
Das Zen Of Python sagt: "Better explicit than implicit". Mit der Verwendung der ``os.path`` Funktionen machst du deutlich, dass du mit Pfaden arbeitest, und nicht mit x-beliebigen Zeichenketten. Dein Code ist schlechter zu lesen als der von birkenfeld gepostete, bei dem jeder weiß, was passiert. Bei deinem Code dagegen muss man erstmal kurz überlegen, welche Strings zu da konkatenierst.

Wenn du, wie in Zeile 116 überprüfen willst, ob eine Liste leer ist, nutze die Tatsache, dass leere Listen im bool'schen Kontext als ``False`` ausgewertet werden: Statt ``if len(dl) == 0:`` also ``if not dl:``.
Naja, das ist wohl eher Geschmackssache, genauso wie manche lieber i=i+1 statt i+=1 schreiben.
Zusätzlich zu dem, was Leonidas gesagt hat, möchte ich dir auch noch die Lektüre von PEP 8 nahelegen.
lunar hat geschrieben: Bezüglich Zeile 133: Hast du das Programm schon mal mit Dateinamen, die Leerzeichen oder Anführungszeichen enthalten, ausprobiert? Du wirst feststellen, dass das nicht funktioniert. ``os.system`` nutzt die Shell zum Starten des Unterprozesses, ergo musst du korrekt quoten. Am besten nimmst du aber gleich das ``subprocess``-Modul.
Leerzeichen oder Anführungszeichen in Dateinamen?
Wer macht den sowas?
Solche Unsitten gibt's bei mir nicht.....
Offensichtliche Bugs absichtlich in Kauf zu nehmen zeugt von sehr geringen Fähigkeiten als Programmierer. Ich hoffe, du hast das nicht ernst gemeint ...
Bezüglich Zeile 161: ``elif`` existiert.
Naja, bei 2 if-Zweigen kann man das doch verkraften ;)
Du solltest dir angewöhnen, die Features der Sprache auszunutzen.
lunar hat geschrieben:Wenn du schon in der stop-Funktion SIGTERM zum Töten des Prozesses nutzt, dann solltest du das auch richtig abfangen, um den Prozess sauber zu beenden.
Und wie sollte das dann genau aussehen?
Weiß nicht warum das nicht reichen sollte, aber ich lasse mich da gerne aufklären...
Du erzeugst einen Traceback, dein Programm stürzt also gewissermaßen ab, und zwar an einer unvorhersehbaren Stelle. Wenn du SIGTERM abfängst, kannst du Aufräumaktionen durchführen, und den Dämon sauber beenden. In deinem Skript passiert zwar noch nichts großartiges, wenn du SIGTERM nicht abfängst, du solltest dir aber gleich angewöhnen, sauber zu programmieren.
Antworten