[Verschönerung] Zeile in Datei ändern

[JonasR]

Hey,

habe heute mal nur eine verbesserungs Frage.
Und zwar möchte ich z.B. die IP von eth0 in der "/etc/network/interfaces" ändern. Dies sieht bei mir momentan wie folgt aus:

Code: Alles auswählen

selected_interface = "eth0"
config_setting_name = "address"
new_setting = "169.254.0.1"

config_file_handle = open("/etc/network/interfaces", "r")
config_file_content = config_file_handle.readlines()
config_file_handle.close()

iface_found = 0
line_counter = 0
for line in config_file_content:
    if "iface " + selected_interface in line:
        iface_found = 1
    if config_setting_name in line and iface_found:
        config_file_content[line_counter] = config_setting_name + " " + new_setting + "\n"
    line_counter += 1
config_file_handle = open("/etc/network/interfaces", "w")
config_file_handle.writelines(config_file_content)
config_file_handle.close()

Nun stellt sich mir die Frage ob das Ganze auch "eleganter" geht... Zum Beispiel ob ich die derzeitige Zeile beim Durchlauf nicht auch anders herausbekomme als mit meinem ""line_counter"".

Btw es handelt sich um Python 2.5 auf einem Debian System. Zusätzliche Module möchte ich eher nicht installieren.

[Hyperion]

Dein Freund dürfte da die Funktion `enumerate` sein Die kann (sollte) man immer dann nutzen, wenn man zusätzlich zum Objekt auch einen passenden Index benötigt.

Generell solltest eher auf `readlines` verzichten. Man kann direkt über die Zeilen einer Textdatei iterieren.

Code: Alles auswählen

with open() as infile:
    for index, line in enumerate(infile):
        print index, line

Das Schlüsselwort `with` sollte es unter 2.5 durch einen future-Import geben iirc.

Außerdem würde ich den String nicht durch "+" zusammenbauen, sondern eine Stringformatierungsmethode nutzen. Ich weiß grad nicht, ob `"".format()` schon in 2.5 verfügbar war, ansonsten eben per `"" % ()` Idiom.

[JonasR]

Hallo Hyperion,
erstmal Danke für die Antwort/den Denkanstoß
'enumerate' kannte ich vorher gar nicht, das werde ich nun öfter mal benutzen
'readlines' habe ich soeben aus meinem Kopf gelöscht nur müsstest du mir noch sagen wie ich ohne weiter komme. Im Voraus mir ist klar das der Code unten nicht funktioniert Nur ist mir nicht klar wie ich den ganzen Text danach wieder in die Datei bekomme. Soll ich jede Zeile in ein Array schreiben und dann per 'writelines' einfügen oder geht es auch anders?

Code: Alles auswählen

from __future__ import with_statement
selected_interface = "eth0"
config_setting_name = "address"
new_setting = "169.254.0.1"
iface_found = 0
with open("/etc/network/interfaces") as config_file:
    for index, line in enumerate(config_file):
        if "iface " + selected_interface in line:
            iface_found = 1
        if config_setting_name in line and iface_found:
            config_file[index] = "%s %s \n" % (config_setting_name, new_setting)

'with' kannte ich schon wusste aber gar nicht dass ich es auch in Python 2.5 einsetzen kann =)
Und beim zusammensetzen des Strings war ich wohl ein wenig schlampig Normalerweise mache ich es mit '"" % ()'

[Hyperion]

Da Du die Datei ändern willst, musst Du Dir alle Zeilen "merken". Das kann man natürlich in einer Liste machen, man kann aber auch eine Generatorfunktion schreiben, die einem die passenden Zeilen zurückliefert. Diese kann man dann auch on-the-fly zu einem String zusammenbauen:

Code: Alles auswählen

def iparse_config_file(path):
    with open() as h:
        for line in h:
            if line...:
                 # mache was mit line
                yield line
            else:
                yield line

new_interfaces = "".join(iparse_config_file("/etc/..."))

So in dieser Art. Den String kann man dann ja einfach in eine Datei wieder wegschreiben. Man könnte das "Parsen" auch noch in einer andere / mehrere Funktion packen, je nach dem, wie aufwendig und modular das ganze sein soll.

Auf jeden Fall würde ich den meisten Code von Modulebene verbannen und eine `main`-Funktion einbauen.

[JonasR]

Code: Alles auswählen

from __future__ import with_statement

selected_interface = "eth0"
config_setting_name = "address"
new_setting = "169.254.0.200"
iface_found = 0
suchstring = "iface %s" % str(selected_interface)
path = "/etc/network/interfaces"

def iparse_config_file(path):
    iface_found = 0
    with open(path) as h:
        for line in h:
            if suchstring in line:
                iface_found = 1
            if iface_found == 1 and "address" in line:
                line = "%s %s\n" % (config_setting_name, new_setting)
                iface_found = 0
            yield line

new_interfaces = "".join(iparse_config_file(path))
with open(path, "w") as handle:
    handle.write(new_interfaces)

Das ist dabei raus gekommen. Funktioniert auch Natürlich wird die Struktur im späteren Code anders aussehen.
Auch wenn ich einiges dazugelernt habe weniger Code ist es jetzt nicht Aber besser als vorher wohl allemal.

[EyDu]

Hallo.

Statt 0 und 1 solltest du `False` und `True` verwenden. Der Sinn von `iface_found` ergibt sich mir auch nicht so ganz, den Test kannst du auch einfach im zweiten if unterbringen. Außerdem ist das `iface_found` auf Modulebene nicht das selbe `iface_found` wie in der Funktion, da der lokale Name den globalen überdeckt.

Sebastian

Edit: Man sich mit der `writelines`-Methode auf `file`-Objekten das `join` sparen.

[Hyperion]

Na, da ist noch Luft nach oben...

Code: Alles auswählen

from __future__ import with_statement

# Das würde ich - da reine Daten - in ein dict packen
selected_interface = "eth0"
config_setting_name = "address"
new_setting = "169.254.0.200"
# wozu steht das noch hier?
iface_found = 0
suchstring = "iface %s" % str(selected_interface)
# Wenn es sich hier um eine Modulkonstante handelt, würde ich path groß schreiben
path = "/etc/network/interfaces"

def iparse_config_file(path):
    # Ein Flag würde ich als Boolean implementieren
    iface_found = 0
    # h ist ein unschöner Name...
    with open(path) as h:
        for line in h:
            # Das  Flag ist doch unnötig, wenn man die Bedingung im selben Durchgang braucht
            # da kann man doch beide Bedingungen in einer formulieren und mit and verknüpfen
            if suchstring in line:
                iface_found = 1
            if iface_found == 1 and "address" in line:
                line = "%s %s\n" % (config_setting_name, new_setting)
                iface_found = 0
            yield line

# Code auf Modulebene - meist keine gute Idee!
new_interfaces = "".join(iparse_config_file(path))
with open(path, "w") as handle:
    handle.write(new_interfaces)

Mein Vorschlag sähe wohl so aus:

Code: Alles auswählen

#!/usr/bin/env python
# coding: utf-8

#from __future__ import with_statement

CONFIG = {
    "interface": "eth0",
    "setting_name": "address",
    "new_address": "192.254.0.200"
}
SEARCH_PATTERN = "iface %s" % CONFIG["interface"]
PATH = "/etc/network/interfaces"

def iparse_config_file(path, config, pattern):
    with open(path) as infile:
        for line in infile:
            if pattern in line and "address" in line:
                line = "%s %s\n" % (config["setting_name"],
                                    config["new_address"])
                # Python ab 2.6
                # line = "{setting_name} {new_address}\n".format(**config)
            yield line

def main():
    new_interfaces = "".join(iparse_config_file(PATH, CONFIG, SEARCH_PATTERN))
    with open(path, "w") as outfile:
        handle.write(new_interfaces)

if __name__ == "__main__":
    main()

[JonasR]

@EyDu
Den Vorschlag mit False und True habe ich mal umgesetzt
'iface_found' wird gebraucht da erst eine Zeile mit dem Interface Namen kommt und in der nächsten dann die Adresszeile. Die Adresszeile kann aber bei verschiedenen Interfaces auftauchen.

@Hyperion

Da ich das ganze nicht als "Standalone-Script" benutzen werde und die 'config' bereits in Variablen geschrieben ist ist denke ich mal diese Variante angebracht:

Code: Alles auswählen

from __future__ import with_statement

def iparse_config_file(path, interface, setting_name, setting_value):
    iface_found = False
    with open(path) as h:
        for line in h:
            
            if "iface %s" % str(interface) in line:
                iface_found = True
            elif iface_found and "address" in line:
                line = "%s %s\n" % (setting_name, setting_value)
                iface_found = False
            yield line

path = "/etc/network/interfaces"
new_interfaces = "".join(iparse_config_file(path, "eth0", "address", "169.254.0.240"))
with open(path, "w") as handle:
    handle.write(new_interfaces)

Btw dein 'if' lässt sich auf einen Fehler von mir zurückführen "iface eth0" und "address" stehen nicht in der selben Zeile

[problembär]

Ich finde das alles kein bißchen schöner als die ursprüngliche Version, aber die Schönheit liegt im Auge des Betrachters.

[Hyperion]

Ich finde das alles kein bißchen schöner als die ursprüngliche Version, aber die Schönheit liegt im Auge des Betrachters.

Mag ja sein, dass "schön" nicht das richtige Wort für die Frage des OP war. "Idiomatischer" ist es jetzt definitiv. Immerhin sind einige Unzulänglichkeiten ausgemerzt worden. Oder willst Du das alles in Abrede stellen? (Stringkonkatenation mittels "+", Öffnen von Dateien ohne `with`, boolesches Flag mit Integer, Code auf Modulebene)

[problembär]

Na ja, wie ich schon ein paarmal geschrieben habe, ist Python eine Sprache, in der besonderer Wert darauf gelegt wird, daß der Programmierer den Code schnell schreiben, lesen und verstehen kann. Um den Preis insbesondere von geringerer Ausführungsgeschwindigkeit.

Strings mit " + " zusammenzufügen, finde ich z.B. sehr leserlich.

Diese "with"-Konstruktion beim Öffnen von Dateien finde ich dagegen eher verwirrend: "You do something with what???".

Daher sind solche Sachen für mich eher Verschlimmbesserungen.
Aber wie gesagt: "Your mileage may vary".

Gruß

[Hyperion]

Na ja, wie ich schon ein paarmal geschrieben habe, ist Python eine Sprache, in der besonderer Wert darauf gelegt wird, daß der Programmierer den Code schnell schreiben, lesen und verstehen kann. Um den Preis insbesondere von geringerer Ausführungsgeschwindigkeit.

Prinzipiell nicht verkehrt, aber erstens ist eine geringere Geschwindigkeit ja kein Ziel oder gar Selbstzweck und zum anderen ist das Lesen und Verstehen durchaus auch abhängig von "gängigen" Mustern. Und dazu gehören nun einmal Konstrukte wie das Öffnen mit `with`, Stringformatierungen usw.

Strings mit " + " zusammenzufügen, finde ich z.B. sehr leserlich.

Neben der Laufzeitverlängerung und dem ungünstigen Speicherverbrauch mal abgesehen¹, wirkt ein solcher String auf mich immer zu stark zerstückelt. Speziell das empfinde ich unter Java bspw. immer total verworren. Und ab 2.6 ist doch die `"".format`-Methode eine feine Sache. Speziell bei "benannten" Parametern liest sich das doch super. Letztlich vergleichbar mit Template-Sprachen.

¹ Die hier bei so kleinen Strings sicherlich kaum eine Rolle spielen. Nur gewöhnt man sich dann bei anfälligen Codepassagen so ein Vorgehen erst gar nicht an, wenn man es auch im kleinen "richtig" macht.

Diese "with"-Konstruktion beim Öffnen von Dateien finde ich dagegen eher verwirrend: "You do something with what???".

Alleine die Einrückungsebene macht es doch viel klarer, dass da ein "besonderes" Objekt eine Rolle spielt. Und "you do something with the explicitly named object at the end of the line" Expliziter und damit leserlicher geht es doch kaum. Zudem spart man sich den Aufruf von `file.close`, welcher ansonsten streng genommen erst in einem finally-Ast untergebracht werden müsste.

Code: Alles auswählen

try:
    infile = open()
    # ganz viel Code
finally:
    infile.close()

Das findest Du wirklich leserlicher?

Sicherlich sind Geschmäcker unterschiedlich; aber leserlich ist einfach nichts, was Du rein individuell betrachten kannst. Wieso würden wir sonst hier alle immer auf PEP8 "bestehen"? Alleine weil solche Konventionen mehrheitsfähig sind und damit vielen Entwicklern das Lesen vereinfachen. Sicherlich mag es da Individualisten geben, die das anders sehen - aber tendenziell prägen nun mal einheitliche Formulierungen beim späteren Lesen anderer Codes das bessere Verständnis.

Sieh es mal so: Im Deutschen gibt es auch zig Vokabeln für gleiche Dinge; nimmt man ein fremdartiges wundert sich der Leser eher, als bei einem gängigen. "Ungebildete" Leser könnten dann tatsächlich sogar den Sinn eines Textes nicht mehr verstehen. Nicht umsonst schneidet Deutschland beim Pisa-Test gerade beim Textverständnis so schlecht ab

So, und nun gehe ich mal schnell in die Kaufhalle, um meiner Muse etwas mundendes zur Abendspeise darreichen zu können. Hoffentlich finde ich alle Ingredenzien für eine leckere Tunke

[snafu]

Die Funktionsbenennung finde ich nicht gerade eindeutig. Etwas wie `get_interfaces()` wäre da schon klarer.

Außerdem halte ich es für unschön, eine `line` als schlichte Zeichenkette zurückzugeben und in bestimmten Fällen sogar eine zu "simulieren". Besser wäre doch der umgekehrte Weg: Die Zeile in einen Tupel zu parsen und diesen dann zurückzugeben. Die Präsentation sollte hier IMHO nicht in der besagten Funktion passieren. In dem Fall würde dann auch wieder der bisherige Name besser passen.

Das Schreiben möchte man dann eventuell in eine zweite Funktion stecken, welche dementsprechend die Aufbereitung der erhaltenen Daten zur Darstellung in der Ausgabe-Datei übernimmt. Sie könnte dies mittels `'\n'.join('{0} {1}'.format(name, value) for name, value in config_info)` vorbereiten und dabei einen Iterator erwarten, der durch die vorgenannte Funktion geliefert wird. Den fertigen String schreibt sie dann ins Outfile.

[Hyperion]

Die Funktionsbenennung finde ich nicht gerade eindeutig. Etwas wie `get_interfaces()` wäre da schon klarer.

Hm... finde ich nicht! Denn die Funktion soll ja nicht die Interfaces finden Natürlich handelt es sich um die "interfaces"-Config-Datei, aber da interfaces auch darin eine Rolle spielen, empfinde ich das als verwirrend.

Außerdem halte ich es für unschön, eine `line` als schlichte Zeichenkette zurückzugeben und in bestimmten Fällen sogar eine zu "simulieren". Besser wäre doch der umgekehrte Weg: Die Zeile in einen Tupel zu parsen und diesen dann zurückzugeben.

Was meinst Du mit simulieren? Kann es sein, dass Du die Aufgabe missverstanden hast? Es geht ja gerade um die Manipulation einer bestimmten Zeile in dieser Konfigurationsdatei. Natürlich kann ich da stumpf ein `split` drauf werfen und habe dann lauter Listen, statt eines Strings mit einer Zeile. Aber: Semantisch hast Du dann noch lange keine gute Abbildung geschaffen, da die Syntax da durchaus komplexer ist als reine Schlüssel-Wert-Paare. Willst Du da also wirklich ein Parsing durchführen, welches die semantische Struktur erfasst, musst Du da natürlich wesentlich komplexer rangehen. Aber das ist ja hier gar nicht gefordert!

Die Präsentation sollte hier IMHO nicht in der besagten Funktion passieren. In dem Fall würde dann auch wieder der bisherige Name besser passen.

Was meinst Du da mit Präsentation? Ich sehe da keine Präsentation. (Ich verstehe darunter einer Ausgabe in irgend einer Form)

Das Schreiben möchte man dann eventuell in eine zweite Funktion stecken, welche dementsprechend die Aufbereitung der erhaltenen Daten zur Darstellung in der Ausgabe-Datei übernimmt. Sie könnte dies mittels `'\n'.join('{0} {1}'.format(name, value) for name, value in config_info)` vorbereiten und dabei einen Iterator erwarten, der durch die vorgenannte Funktion geliefert wird. Den fertigen String schreibt sie dann ins Outfile.

Sicher; ich hatte das in der main()-Funktion untergebracht, einfach weil es so simpel ist und ich es von der Modulebene haben wollte. Der OP hat aber ja gesagt, dass er das ganze eh nur als POC ansieht und die Konzepte in ein anderes, größeres Script übernehmen will. Ok, ich würde das auch bei so einer simplen Sache nicht auf Modulebene stehen lassen
Der OP benutzt Python 2.5 - gibt es da schon `"".format` per future-Import?

So, ich habs noch mal ein wenig besser strukturiert: Link

[snafu]

Okay, ich habe die Aufgabe gestern wirklich nicht mehr ordentlich gelesen. Ein Parsen ist hier in der Tat etwas zuviel des Guten. Ehrlich gesagt würde ich aber einfach `re.sub()` in der `change_address()` benutzen. In seiner jetzigen Form halte ich das Vorgehen für eher umständlich.

[Hyperion]

Okay, ich habe die Aufgabe gestern wirklich nicht mehr ordentlich gelesen. Ein Parsen ist hier in der Tat etwas zuviel des Guten. Ehrlich gesagt würde ich aber einfach `re.sub()` in der `change_address()` benutzen. In seiner jetzigen Form halte ich das Vorgehen für eher umständlich.

Inwiefern umständlich? Klar kann man das auch mit anderen Methoden lösen, aber da diese Zeile ja sehr einfach aufgebaut ist, spricht doch imho nix gegen ein komplettes Generieren einer neuen Zeile. Und einen anderen Vorteil bieten RegExps bei diesem Anwendungsfall doch nicht - oder sehe ich hier etwas nicht?

[snafu]

Beide Methoden führen zum selben Ziel. Eventuell finden manche Leute die zugehörige Regex auch wiederum zu umständlich zu lesen. Das ist Ansichtssache und hat ein stückweit sicher auch mit den eigenen Kenntnissen zu tun, die man als Hintergrund mitbringt.

Also ich kenne den genauen Aufbau der Datei nicht, hätte aber etwas in dieser Art vorgeschlagen:

Code: Alles auswählen

re.sub(r'(?<=address )(?:\d{1,3}\.){3}\d{1,3}', new_ip, file_content)

Natürlich ist das viel zu simpel, um den ganzen Dateiinhalt richtig zu matchen. In tatsächlich verwendetem Quelltext würde man wohl auch noch ein paar Kommentare dazu einbauen. Wie gesagt: Nur ein Vorschlag.

EDIT:

Ausgehend davon, dass keine großartige Prüfung des gegebenen IP-Formates notwendig ist, könnte man den Ausdruck sogar noch etwas simpler gestalten:

Code: Alles auswählen

r'(?<=address )(\d|\.)+'

[Hyperion]

Damit kannst Du aber nur Zeile 30 in meinem Vorschlag ersetzen - denn Du berücksichtigst nicht, dass Du erst einmal eine "Sektion" eines bestimmten Interfaces finden musst und nur darin die Adresse ändern sollst. Du kannst Deinen RegExp also nur auf eine bestimmte Zeile anwenden und nicht auf die gesamte Datei. Aus diesem Grund sehe ich hier keinen Vorteil eines RegExps, da das "Schlüsselwort" `address` vor dem dynamischen Teil ja fix ist und daher imho auch simpel zusammengebaut werden kann.

[JonasR]

Mal als Beispiel

Code: Alles auswählen

auto lo
iface lo inet loopback
	address 127.0.0.1
	netmask 255.0.0.0
	broadcast 127.255.255.255
	up ip route replace 127.0.0.0/8 dev lo

auto eth0
iface eth0 inet static
	address 192.168.0.1
	netmask 255.255.255.0
	broadcast 0.0.0.0

auto eth1
iface eth1 inet static
	netmask 255.255.0.0
	address 169.254.0.1

€ Also ist die /etc/network/interfaces

[snafu]

Okay, hier muss ich passen. Das Lookbehind mag keine Zwischenräume von variabler Länge (d.h. den Bereich von `iface eth0` bis `address`). Das wusste ich nicht. Ich weiß daher nicht, ob die Bearbeitung in einem Rutsch tatsächlich noch möglich ist.