Man kann Funktionen auch leichter einzeln testen und wiederverwenden wenn man dafür keinen globalen Zustand berücksichtigen muss.
Verwirrend wird es auch wenn man irgendwann mal den Namen einer globalen Variable für eine lokale Variable verwendet. Sowohl beim Schreiben als auch beim Lesen muss man auf so etwas dann aufpassen.
Re Datenbank: Wenn die zweite Datei nicht da ist dann ist die erste Datei eben noch nicht in der Datenbank, denn solange die Transaktion nicht durch ein commit abgeschlossen ist, sind die Änderungen für Lesezugriffe noch nicht sichtbar und können wieder zurückgerollt werden. Das ist die Funktion von Transaktionen, dass man mehrere Änderungen machen kann, die entweder in ihrer Gesamtheit in der Datenbank landen oder gar nicht. Das auf Dateiebene selbst zu programmieren ist umständlich bis unmöglich wenn es portabel sein soll.
Du packst das auswefen des Sticks immer an komische/falsche Stellen. Die Funktion die das Anstecken des Sticks erkennen soll, sollte das Anstecken des Sticks erkennen und den Namen liefern. Und nichts anderes. Deren Aufgabe ist weder das Prüfen auf vorhandensein von Dateien noch das Auswerfen des Sticks. Das beim kopieren machen zu wollen war ja auch falsch. Das eine Funktion die `copy()` heisst und deren Aufgabe es ist *eine* Datei zu kopieren, auch den Stick auswerfen kann ist unerwartet und wie sich herausgestellt hat auch falsch, denn diese Entscheidung kann man dort überhaupt nicht treffen, weil da die nötigen Informationen fehlen. Zum Beispiel das danach vielleicht noch ein `copy()`-Aufruf kommen könnte, der ohne Stick natürlich auf die Nase fällt.
Funktionsnamen sollten die Tätigkeit beschreiben die sie durchführen. `usb_ansteckerkenner()` und `auswerfer()` sind aber keine Tätigkeiten sondern eher Namen für ”Dinge”, also eher passivere Objekte. `warten_auf_usb_stick()` und `auswerfen()` wären passender.
`timestamp` sollte nach dem warten auf den Stick ermittelt werden.
Python hat einen eigenen Datentyp für Wahrheitswerte (`bool`) mit den Werten `True` und `False`, dafür sollte man nicht die Zahlen 1 und 0 missbrauchen.
`gehts_weiter` ist überflüssig. Man muss nicht jedes Ergebnis an einen Namen binden.
Das `usb_ansteckerkenner()` nicht der geeignete Ort ist um den Stick auszuwerfen sieht man auch daran das die Funktion ja den Namen zurück gibt. Wenn Du den Stick auswirfst, dann tut sie das aber nicht, sondern gibt implizit `None` zurück, was der Code an der Aufrufstelle dann versucht als Namen für den Stick zu verwenden, was natürlich zu einer weiteren Ausnahme führen wird. Was zu einem erneuten Versuch den bereits ausgeworfenen Stick erneut auszuwerfen, was wieder zu einer Ausnahme führt. Die dann das Programm beendet. Das ist alles sehr verwirrend strukturiert.
Wobei ich gerade sehe das `copy()` im Falle einer nicht gefundenen Datei gar keine Ausnahme auslöst, der Aufrufer also gar keine Möglichkeit hat darauf zu reagieren wenn das der Fall ist. Die Ausnahmebehandlung dort ist keine sinnvolle Ausnahmebehandlung. Einen Fehler einfach ausgeben und dann so weiter machen als sei alles in Ordnung ist sehr selten ein sinnvoller Umgang mit Ausnahmen. Und wenn man eine Ausnahme nicht sinnvoll behandeln kann, sollte man sie an der Stelle einfach gar nicht behandeln.
Das implizite `None` sollte keine Funktion zurückgeben, die mindestens eine ``return``-Anweisung enthält. Falls so eine Funktion auch `None` als Rückgabewert haben kann, dann sollte das explizit mit einem ``return None`` passieren, damit der Leser das deutlich sieht, und weiss das es Absicht ist und kein Versehen weil ein Ablaufpfad in der Funktion übersehen wurde.
`pruef_ob_da()` gibt entweder 1 zurück oder auch wieder ein implizites `None`. Das sollte aber `True` und `False` sein. Im `None`-Fall wird eine Ausgabe für den Benutzer gemacht – so etwas hat in so einer Funktion nichts zu suchen.
Die hart kodierte 2 sollte da nicht stehen. Das ist die Länge der Liste mit den zu prüfenden Dateien. Wenn man die Länge ändert, an ganz anderer Stelle im Code, muss man daran denken in dieser Funktion diesen ”magischen” Wert zu ändern.
Das mit dem `x` und zählen ist auch umständlich und ineffizient. Es ist doch egal wie viele Dateien vorhanden sind, wichtig ist das alle vorhanden sind. Dazu muss man nix zählen sondern bei der ersten nicht vorhandenen Datei ist klar das nicht alle vorhanden sind. Dann steht das Ergebnis fest, auch wenn man noch nicht alle Dateien getestet hat.
Die Funktion könnte dann so aussehen:
Code: Alles auswählen
def pruef_ob_da(name_of_stick):
for dateiname in map(Path, WECHSEL_DATEI_NAMEN):
if not (MEDIA_PFAD / name_of_stick / dateiname).is_file():
return False
return True
Code: Alles auswählen
def pruef_ob_da(name_of_stick):
return all(
(MEDIA_PFAD / name_of_stick / dateiname).is_file()
for dateiname in map(Path, WECHSEL_DATEI_NAMEN)
)
Bei Deinem Code erfolgt das Auswerfen des Sticks übrigends *nicht* wenn alle kopiert werden konnte. Soll das so? Ich hätte ja jetzt eher gedacht das es egal ist alles kopiert werden konnte oder nicht, das am Ende der Stick ausgeworfen werden soll‽
Warum bekommt `copy()` den Namen des Sticks? Der wird doch da gar nicht verwendet‽
Ungetestet:
Code: Alles auswählen
#!/usr/bin/env python3
import subprocess
import time
from datetime import datetime as DateTime
from pathlib import Path
import pyudev
PFAD = Path.home() / ".DruckData"
MEDIA_PFAD = Path("/media/earl/")
WECHSEL_DATEI_NAMEN = ["numbers.csv", "TB_Ausgabe_8iii.txt"]
def warte_auf_usb_stick(udev_context):
monitor = pyudev.Monitor.from_netlink(udev_context)
monitor.filter_by("block")
for device in iter(monitor.poll, None):
if "ID_FS_TYPE" in device and device.action == "add":
name_of_stick = Path(device.get("ID_FS_LABEL"))
time.sleep(2)
return name_of_stick
raise AssertionError("unreachable code")
def copy(source_path, destination_path):
text = source_path.read_text(encoding="utf-8")
destination_path.write_text(text, encoding="utf-8")
def datei_auf_stick(name_of_stick, dateipfad, timestamp):
copy(
PFAD / dateipfad,
MEDIA_PFAD
/ name_of_stick
/ dateipfad.with_name(
f"{dateipfad.stem}_{timestamp:%Y-%m-%d_%H_%M}.csv"
),
)
def datei_auf_arbeitsverzeichnis(name_of_stick, dateipfad):
copy(MEDIA_PFAD / name_of_stick / dateipfad, PFAD / dateipfad)
def auswerfen(name_of_stick):
subprocess.run(
["umount", MEDIA_PFAD / name_of_stick],
check=True,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
)
def main():
udev_context = pyudev.Context()
while True:
name_of_stick = warte_auf_usb_stick(udev_context)
timestamp = DateTime.now()
try:
#
# TODO Nicht direkt die lokalen Dateien überschreiben sondern erst
# alle mit temporären Namen kopieren und dann umbenennen um
# wenigstens zu versuchen zu verhindern das es nur ein teilweises
# Update gibt.
#
for dateiname in map(Path, WECHSEL_DATEI_NAMEN):
datei_auf_stick(name_of_stick, dateiname, timestamp)
datei_auf_arbeitsverzeichnis(name_of_stick, dateiname)
except OSError as error:
print("Fehler beim kopieren:", error)
#
# TODO Soll der Stick tatsächlich nur ausgeworfen werden wenn beim
# kopieren Fehler auftraten?
#
try:
auswerfen(name_of_stick)
except subprocess.CalledProcessError as error:
print(
f"Fehler {error.returncode} beim Auswerfen: {error.stdout}"
)
if __name__ == "__main__":
main()