Log-Datein erstellen

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
JohannesGolf
User
Beiträge: 34
Registriert: Montag 10. März 2014, 09:27

Hallo zusammen,

ich bin gerade in einer Ausbildung zum Fachinformatiker Anwendungsentwicklung und brauch etwas Hilfe bei einem Programm.

Das Programm sucht in einem Verzeichnis (den Pfad kann man eingeben) nach Log-Datein und diese kann man noch nach weitere Kriterien sortieren (Datum und ob ein neuer Pfad erstellt werden soll/ ein bestehender genommen werden soll) und dann werden diese kopiert, die Kopie komprimiert und die ursprüngliche Datei gelöscht.

Und hier brauch ich Eure Hilfe:
Zur Übersicht möchte ich jetzt das das Programm selber eine Log-Datei erstellt und diese speichert.
Wenn das Programm erneut aufgerufen wird soll es die bestehende Log-Datei weiterschieben.

Damit Ihr euch es besser Vorstellen könnt ist hier mein bisheriger Programm-Code:
https://gist.github.com/JohannesGolf/9462793

Ich freu mich auf Eure Antworten und möchte mich auch schon mal dafür bedanken.

Mit freundlichen Grüßen

JohannesGolf
Zuletzt geändert von JohannesGolf am Montag 10. März 2014, 11:40, insgesamt 1-mal geändert.
BlackJack

@JohannesGolf: Schau Dir mal das `logging`-Modul in der Standardbibliothek an.

Und als nächstes vielleicht den Style Guide for Python Code, denn die Schreibweise der Funktionsnamen entspricht nicht den üblichen Konventionen.

Als nächstes sollte das Hauptprogramm in einer Funktion verschwinden und mit dem ``if __name__ == '__main__': main()``-Idiom aufgerufen werden. Dann kann man das Modul importieren ohne dass das Hauptprogramm los läuft. Zum Beispiel um einzelne Funktionen interaktiv zu testen oder wiederzuverwenden.
Benutzeravatar
darktrym
User
Beiträge: 784
Registriert: Freitag 24. April 2009, 09:26

Und benutzte doch bitte lieber die dokumentierten Funktionen statt was eigenes zu basteln.
Pfade werden zusammengeklebt mit os.path.join, wenn du ein alias für os.path.exists brauchst, schreibt man doch keine Funktion.
Leere Strings geben immer 0 zurück, Tipp für Z64 und auf False vergleicht man schon mal gar nicht, Z116.
Print Argumente werden durch Komma getrennt, da brauchst keine Stringarithmetik.

EDIT: Dateierweiterungen prüft man üblicherweise mit sowas wie os.path.splitext[-1].
Zuletzt geändert von darktrym am Montag 10. März 2014, 11:57, insgesamt 3-mal geändert.
„gcc finds bugs in Linux, NetBSD finds bugs in gcc.“[Michael Dexter, Systems 2008]
Bitbucket, Github
Benutzeravatar
Hyperion
Moderator
Beiträge: 7478
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

Und meine Bitte bei so langem Code ist stets die folgende: Lagere den Code in ein Pastebin aus; wir haben ein eingebautes im Forum oder ansonsten gist.github.com. :-)
encoding_kapiert = all(verstehen(lesen(info)) for info in (Leonidas Folien, Blog, Folien & Text inkl. Python3, utf-8 everywhere))
assert encoding_kapiert
BlackJack

@JohannesGolf: Weitere Anmerkungen:

Um eine Leerzeile auszugeben reicht ein ``print``, man muss nicht auch noch ein Leer*zeichen* in der leeren Zeile ausgeben was sowieso niemand sieht.

Die Funktion `conversationWithUser()` ist überflüssig ebenso wie das vorherige Ausgeben von `talk`, denn `raw_input()` kann man das auch als Argument mitgeben. Und dann wäre `conversationWithUser()` letztlich nur ein umbenennen von `raw_input()`. Das gleiche gilt für `doesPathExists()` was doch exakt das selbe macht wie `os.path.exists()`.

Der Test ob der Dateiname zu einer Log-Datei gehört ist zu kompliziert. Zuerst kann man wenn man aufgrund einer ``if``-Bedingung `True` oder `False` zurück gibt, sich das ``if`` sparen, denn die Bedingung selbst ergibt ja schon `True` oder `False`, also kann man die auch einfach auswerten und das Ergebnis als Rückgabewert verwenden:

Code: Alles auswählen

#
# Statt:
# 
def is_log_file(path):
    if path.find('.log') != -1:
        return True
    return False
# 
# Besser:
# 
def is_log_file(path):
    return path.find('.log') != -1
Hier wäre zusätzlich noch anzumerken das der ``in``-Operator irgendwie naheliegender ist als die `find()`-Methode und auch leichter und ”natürlicher” zu lesen ist.

Der Name `formatSeveralDateTime()` erschliesst sich mir nicht? Was bedeutet das `several` hier? Was die Funktion eigentlich tut würde man durch `timestamp_to_date_string()` verständlicher beschreiben.

In `isFileInDateRange()` kann man wieder das unnötige ``if`` einsparen und gleich das Ergebnis der Bedingung zurück geben. Ausserdem ist das ``and`` nicht nötig wenn man die beiden Bedingungen entsprechend verkettet, also so wie man das auch in der Mathematik schreiben würde: ``return start_date <= file_date <= end_date``.

Bei den letzten beiden Funktionen sollte man IMHO nicht mit Zeichenketten arbeiten, denn die eigentlichen Werte auf denen man hier operiert sind ja *Datumsangaben* und dafür gibt es dedizierte Typen im `datetime`-Modul, die man auch Vergleichen kann ohne sie in Zeichenketten umzuwandeln.

Bei der ZIP-Funktion sind nicht alle Namen gut gewählt. `*Content` ist zum Beispiel gar nicht der Inhalt sondern ein Dateiname. Was soll das `my` bei `myzip` bedeuten? Der Prädix `my` ist in aller Regel ein Warnzeichen, dass man einen schlechten Namen vor sich hat, denn das `my` bedeutet nichts, kann also mindestens mal weg gelasssen werden.

Die ZIP-Datei wird in der Funktion nicht geschlossen, was zu Problemen führen kann, denn Dateideskriptoren sind eine Ressource die das Betriebssystem nicht unendlich zur Verfügung stellt. Darum sollte man immer dafür sorgen das Dateien die man öffnet, auch wieder geschlossen werden. Die ``with``-Anweisung hilft dabei das auch in jedem Fall zu tun. Für Objekte die selbst mit ``with`` nicht interagieren aber eine `close()`-Methode besitzen, kann man `contextlib.closing()` verwenden.

Die Dateiendung würde ich von aussen vorgeben lassen und nicht in der Funktion erst anhängen.

Wahrscheinlich waren ZIP-Archive eine Vorgabe in der Aufgabe, aber das ist etwas komisch ein Archiv-Format für Einzeldateien zu verwenden. In der Praxis würde man Einzeldateien mit einem reinen Kompressionsformat wie gzip oder bzip2 behandeln. Die entsprechenden Module dafür gäbe es in der Python-Standardbibliothek.

`repeatUserInputUntilValidInput()` ist ein ziemlich umständlicher Name für etwas was den Benutzer nach 'Y' oder 'N' fragt.

Das `userInput`-Flag in der Funktion ist komplett überflüssig. Wenn die Bedingung der ``while``-Schleife ausgewertet wird, kommt da *immer* `True` bei heraus, also kann man dort auch gleich ``while True:`` schreiben. Es ist auch etwas schräg eine Funktion zu schreiben die schon eine erste Antwort als Argument erwartet, so dass man bei jedem Aufruf der Funktion vorher schon mal ein `raw_input()` stehen haben muss. Dann lassen sich auch einige lokale Variablen für diese erste Eingabe einsparen.

'insert' sollte hier wohl in der Kommunikation mit dem Benutzer 'enter' heissen.

`pathNameLongerThan0()` ist eine eigenartige Funktion. Warum hast Du gerade diesen Teilaspekt in eine Funktion heraus gezogen, statt zum Beispiel eine Funktion zu schreiben die den Benutzer auffordert einen existierenden Pfad einzugeben und das solange wiederholt bis das tatsächlich passiert ist? Damit hätte man mehr sich im Hauptprogramm wiederholenden Code erschlagen und auch eine insgesamt sinnvollere, in sich geschlossene Tätigkeit in einer Funktion gekapselt.

Namen sollten räumlich möglichst nah dort definiert werden, wo sie auch verwendet werden. Das ist übersichtlicher als wenn man erst weit nach oben im Quelltext suchen muss wo ein Name definiert wird und ob und was seit dem mit dem Wert passiert ist. Das betrifft zum Beispiel die Liste in der die Dateinamen der Logdateien gesammelt werden.

Bei der Eingabe der Datumsangaben sind die Flags überflüssig. Die kann man alle drei Einsparen wenn man „Endlosschleifen” schreibt und die im Erfolgsfall mit ``break`` verlässt.

Der Präfix `user` den Datumsangaben macht keinen Sinn.

Der Quelltext für die Eingabe des Startdatums entspricht fast dem für die Eingabe des Enddatums. Solche Code-Wiederholungen vermeidet man besser durch eine Funktion zur Eingabe eines Datums.

Pfade sollte man mit `os.path.join()` zusammensetzen und nicht mit Zeichenkettenoperationen wie ``+`` und hart kodierten Pfadtrennern.

Beim Traversieren der Verzeichnis- und Dateinamen mit `os.walk()` wird `logFolder` neu gebunden, das ist verwirrend das der Name nach der Schleife etwas anderes bedeutet als vorher. `userFolderPath` ist auch nicht so leicht verständlich. Man muss erst in die Dokumentation von `os.walk()` schauen um wenn man nicht aus dem Kopf weiss in welcher Reihenfolge die Funktion durch die Verzeichnisse geht. Ich wüsste das jetzt nicht aus dem Kopf und müsste raten oder nachlesen. Meine Vermutung ist, dass Du da einen Programmierfehler gemacht hast. Der Name ist auch total nichtssagend, vielleicht auch ein Grund warum ich nicht verstehe was das letztendlich bewirken/bedeuten soll.

`file` ist der Name eines eigebauten Typs den sollte man nicht verdecken. Zumal der Wert den Du da bindest auch gar kein Datei-Objekt ist, sondern ein Datei*name*.

Anstelle von ``if len(sequence) > 0:`` kann man auch einfach ``if sequence:`` schreiben.

Das `quit()` am Ende ist überflüssig und auch eigentlich gar nicht möglich, denn diese Funktion gibt es nicht in der Dokumentation. Die ist nur in einer interaktiven Python-Shell garantiert vorhanden.

Das Hauptprogramm ist so noch zu lang, das kann man noch weiter aufteilen. Dabei solle man Programmlogik und Benutzerinteraktion trennen, so dass sie die Programmfunktion auch ohne Interaktion mit einem Benutzer verwenden lässt. Dann kann man nämlich die Funktion einfacher testen, und auch einfacher wiederverwenden. Zum Beispiel könnte man eine Kommandozeilen-API oder eine GUI anbieten, ohne das man alles neu schreiben oder umschreiben muss.

Ich komme ungefähr bei so etwas als Zwischenstand heraus (ungetestet):

Code: Alles auswählen

import os
from contextlib import closing
from datetime import datetime as DateTime
from zipfile import ZIP_DEFLATED, ZipFile

__version__ = '1.4'

INVALID_INPUT_MESSAGE = 'Your input is invalid. Please try again!'


def print_program_info():
    print
    print '#######################################################'
    print 'Python Log-Packer.py  Ver.', __version__
    print 'Search for files, separate the .log fils, compress them'
    print 'and delete the origin file'
    print 'log-File = Files with ".log" in name'
    print '#######################################################'
    print
 

def is_log_file(path):
    return '.log' in path
 

def timestamp_to_date_string(timestamp):
    return DateTime.fromtimestamp(timestamp).strftime('%Y-%m-%d')
 

def if_in_date_range(path, start_date, end_date):
    # 
    # TODO Change this to work with `datetime.date` objects instead of strings.
    # 
    file_date = timestamp_to_date_string(os.path.getmtime(path))
    return start_date <= file_date <= end_date
 

def zip_file(archive_filename, source_filename):
    with closing(ZipFile(archive_filename, 'w', ZIP_DEFLATED)) as zipfile:
        zipfile.write(source_filename)
 

def is_valid_date_string(date_string):
    if len(date_string) != 10:
        return False
    try:
        DateTime.strptime(date_string, '%Y-%m-%d')
        return True
    except ValueError:
        return False  
 

def ask_y_or_n(prompt):
    answer = raw_input(prompt + '  (Y/N): ').upper()
    while True:
        if answer == 'Y':
            return True
        elif answer == 'N':
            return False
        else:
            print INVALID_INPUT_MESSAGE
            answer = raw_input("Please insert 'Y' or 'N'!\n")
 

def ask_for_existing_directory(prompt):
    while True:
        path = raw_input(prompt + ': ')
        if os.path.exists(path) and os.path.isdir(path):
            return path
        print 'Please enter an existing directory!'


def ask_date_string(prompt):
    while True:
        date_string = raw_input(prompt)
        if is_valid_date_string(date_string):
            return date_string
        print INVALID_INPUT_MESSAGE


def main():
    # 
    # TODO Function is too long.  Split up into subfunctions.  Separate
    #   user interaction and program logic.
    # 
    print_program_info()

    while True:
        destination_path = ''
        logfiles_path = ask_for_existing_directory(
            'Enter base path of the log files'
        )
        if ask_y_or_n('Do you want to define a Date Range?'):
            while True:
                start_date_string = ask_date_string(
                    'Please enter the beginning date (e.g. 2014-05-23): '
                )
                end_date_string = ask_date_string(
                    'Please enter the ending date (e.g. 2014-11-03): '
                )
                if start_date_string <= end_date_string:
                    break
                print INVALID_INPUT_MESSAGE
                print 'Date out of Range. Enter again!'
        else:
            start_date_string, end_date_string = '1900-01-01', '2090-01-01'
     
        if ask_y_or_n(
            'Do you want create a new folder or archive the files in another'
            ' folder?'
        ):
            if ask_y_or_n('Do you want to create a new folder?'):
                while True:
                    print INVALID_INPUT_MESSAGE
                    destination_path = raw_input(
                        'Enter a new absolute path please: '
                    )
                    if os.path.isabs(destination_path):
                        break
            else:
                destination_path = ask_for_existing_directory(
                    'Enter destination directory'
                )              
        else:
            destination_path = os.path.join(logfiles_path, destination_path)
     
        print '#######################################################'
        print 'Informations'
        print 'Logfolder:  ', logfiles_path
        print 'Stardate:   ', start_date_string
        print 'Enddate:    ', end_date_string
        print 'Destination:', destination_path
        print '#######################################################'
        if ask_y_or_n('Are those informations correct?'):
            break
        print '#######################################################'  
    #
    # Here starts the compression code.
    # 
    log_filenames = list()
    for base_path, _dirnames, filenames in os.walk(logfiles_path):
        print '#######################################################'
        for filename in filenames:
            full_path = os.path.join(base_path, filename)
            if (
                is_log_file(filename)
                and if_in_date_range(
                    full_path, start_date_string, end_date_string
                )
            ):
                log_filenames.append(full_path)
    # 
    # XXX Potential bug!  Is the last visited path of `os.walk()` really
    #   what is wanted here?
    #   
    #   What about the name of this variable?  What does it *mean*?
    # 
    # FIXME If the folder at `logfiles_path` is empty then `base_path` is
    #   undefined here.
    # 
    user_folder_path = base_path
     
    if log_filenames:
        if destination_path:
            if not os.path.exists(destination_path):
                os.mkdir(destination_path)    
            user_folder_path = destination_path
     
        for log_filename in log_filenames:
            zipfile_path = (
                os.path.join(
                    user_folder_path, os.path.basename(log_filename) + '.zip'
                )
            )
            zip_file(zipfile_path, log_filename)
            print log_filename
            os.remove(log_filename)
    
    print '#######################################################'        
    print 'finished'
    print '#######################################################'
@darktrym: Das mit der Dateiendung hätte ich auch fast geschrieben, aber es gibt auch viele Logfiles die rotiert werden und wo dann nach dem ``.log`` noch ein weiterer Punkt und eine Nummer kommt. Und viele Admins speichern Kopien von Logs mit einem an den Dateinamen angehängten Datum — diese Dateien möchte man vielleicht auch sichern.
JohannesGolf
User
Beiträge: 34
Registriert: Montag 10. März 2014, 09:27

Danke für die Hilfe.

Das mit dem 'Style Guide' wurde mir so, wie ich es im Code oben gemacht habe, im Betrieb und der Schule beigebracht.
BlackJack hat geschrieben:@JohannesGolf: Schau Dir mal das `logging`-Modul in der Standardbibliothek an.

Und als nächstes vielleicht den Style Guide for Python Code, denn die Schreibweise der Funktionsnamen entspricht nicht den üblichen Konventionen.

Als nächstes sollte das Hauptprogramm in einer Funktion verschwinden und mit dem ``if __name__ == '__main__': main()``-Idiom aufgerufen werden. Dann kann man das Modul importieren ohne dass das Hauptprogramm los läuft. Zum Beispiel um einzelne Funktionen interaktiv zu testen oder wiederzuverwenden.
Habs so hinbekommen wie ichs wollte. Der Beitrag kann geschlossen werden.
Zuletzt geändert von JohannesGolf am Dienstag 11. März 2014, 16:18, insgesamt 1-mal geändert.
Benutzeravatar
Hyperion
Moderator
Beiträge: 7478
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

JohannesGolf hat geschrieben:Das mit dem 'Style Guide' wurde mir so, wie ich es im Code oben gemacht habe, im Betrieb und der Schule beigebracht.
Um so trauriger... denn Python gehört zu den Sprachen, bei denen es *einen* akzeptierten Styleguide gibt. Und wie BlackJack schrieb, sollte man sich *unbedingt* an diesen halten.

Denk an das ungeordnete mittelalterliche Deutsch. Damals konnte jeder schreiben, wie er es "hörte". Das mündete letztlich darin, dass das Lesen von alten deutschen Texten sehr schwierig sein kann, da man unterschiedlichste Schreibweisen für ein und dasselbe Wort findet. Bei Programmiersprachen ist es ähnlich gelagert. Natürlich kann ich eine Funktion statt ``get_data`` auch ``getdata``, ``GetData``, ``getData`` oder gar ``Getdata`` oder noch anders schreiben. Wenn sich jedoch jeder an Konventionen hält, ist das Auge und der Verstand viel schneller in der Lage, den Code zu erfassen und richtig zu interpretieren.

Bei Sprachen mit mehreren, konkurrierenden Styleguides (C++ etwa) ist es sicherlich schwieriger, die "richtig" Linie zu finden, wenn gleich man auch da keinen Mischmasch betreiben sollte.

Also, nimm diese Lehre an und belehre stattdessen Deine Lehrer! ;-)
encoding_kapiert = all(verstehen(lesen(info)) for info in (Leonidas Folien, Blog, Folien & Text inkl. Python3, utf-8 everywhere))
assert encoding_kapiert
JohannesGolf
User
Beiträge: 34
Registriert: Montag 10. März 2014, 09:27

Hyperion hat geschrieben:
JohannesGolf hat geschrieben:Das mit dem 'Style Guide' wurde mir so, wie ich es im Code oben gemacht habe, im Betrieb und der Schule beigebracht.
Um so trauriger... denn Python gehört zu den Sprachen, bei denen es *einen* akzeptierten Styleguide gibt. Und wie BlackJack schrieb, sollte man sich *unbedingt* an diesen halten.

Denk an das ungeordnete mittelalterliche Deutsch. Damals konnte jeder schreiben, wie er es "hörte". Das mündete letztlich darin, dass das Lesen von alten deutschen Texten sehr schwierig sein kann, da man unterschiedlichste Schreibweisen für ein und dasselbe Wort findet. Bei Programmiersprachen ist es ähnlich gelagert. Natürlich kann ich eine Funktion statt ``get_data`` auch ``getdata``, ``GetData``, ``getData`` oder gar ``Getdata`` oder noch anders schreiben. Wenn sich jedoch jeder an Konventionen hält, ist das Auge und der Verstand viel schneller in der Lage, den Code zu erfassen und richtig zu interpretieren.

Bei Sprachen mit mehreren, konkurrierenden Styleguides (C++ etwa) ist es sicherlich schwieriger, die "richtig" Linie zu finden, wenn gleich man auch da keinen Mischmasch betreiben sollte.

Also, nimm diese Lehre an und belehre stattdessen Deine Lehrer! ;-)
Den Lehrer belehren, ok wird gemacht. Ich freu mich schon auf sein Gesicht.
Antworten