Werte an ein Shell Script übergeben

[Sirius3]

@Kalli87: Programmieren heißt zu allererst, ein Problem in sinnvolle Teilprobleme aufzuspalten. Das erste Teilproblem, das Du bisher noch nicht gelöst hast, ist dieses grauenvolle Skript aufzurufen und mit den richtigen Werten zu füttern. Dazu überlegst Du Dir zuerst, welche Parameter Du brauchst, also Patientennummer alt, neu usw. Dann schreibst Du eine Funktion, die diese Parameter braucht und füllst den Rumpf dieser Funktion mit dem popen-Kram. Dann testest Du diese Funktion.

Und wenn Du das alles gemacht hast, kannst Du Dir überlegen, wie Du diese Funktion aus Deiner GUI heraus aufzurufen ist.

[Hyperion]

Ach Gottchen... diese Umsetzung ist ja konzeptionell der größte Mist überhaupt! Man soll also eine GUI (Python) schreiben, die eine andere Form von UI (TUI in einem Bash-Script) ansteuern kann, da diese die eigentliche Funktionalität (Whatever) anstößt.

@Kalli87: Dein Ausbilder macht sich keine wirklichen Gedanken...

[Kalli87]

@Hyperion: Das hängt eher damit zusammen hier viel mit Scripten gearbeitet wird und keiner so richtig programmieren kann. (So siehts jedenfalls für mich aus)
Da das Script richtig funktioniert soll halt eine GUI Entwickelt werden die das ganze Script dann ansteuert, klingt für mich auch blödsinnig aber es ist nun mal so. Wenn jetzt einer auf die Idee kommt das am Script was geändert werden muss können sie das ja einfach tun, muss halt nur beachtet werden das die GUI am ende das Script weiterhin richtig ansteuert.

@Sirius3: Naja mit Shell=TRUE hatte ichs ja schon aber BlackJack meint ja das gerade dass das nicht gerade elegant is, mittlerweile hab ichs auch so gelöst dass das Script läuft.

[Sirius3]

@Kalli87: nimm's nich übel, wenn hier kaum jemand glaubt, dass Du eine Lösung gefunden hast. Dass etwas läuft und dass etwas einigermaßen robust und fehlerfrei läuft, sind zwei grundverschiedene Dinge.

[Kalli87]

@Sirius3: ich nehm nichts übel keine sorge, ich hab ja auch nur gesagt das ich geschafft habe dass das Script läuft aber nicht das ich auch schon mit Eingaben füttere.
Ich versuche mich an das zu halten was du geschrieben hast, Teilprobleme lösen, und dann sich das nächste Problem anzunehmen.

Code: Alles auswählen

import subprocess

proc = subprocess.Popen(['bash', '/DVMERGE/DVMERGE.sh'])
proc.communicate()

[BlackJack]

Das Bash-Skript selbst ist ja schon ziemlich übel. Das hat wohl auch ein Praktikant verfasst.

Die alte und die neue Patientennummer sind im Skript jeweils an zwei verschiedene Namen gebunden und beide Varianten werden verwendet. WTF‽

Es werden keine Überprüfungen bei der Patientennummer durchgeführt, also zum Beispiel das sie nicht zu lang wird oder das sie nur aus Ziffern bestehen darf.

Nach der Abfrage ob man die Nummer ersetzen möchte, macht das Skript erst einmal einen Haufen anderer Sachen bevor diese Antwort ausgewertet wird.

Die AWK-Programme sind alle fehlerhaft, nur das der Fehler zufällig nichts ausmacht. Es wird dort so etwas wie ``$QUELL_PATNU`` benutzt was aber mit der Variablen `$QUELL_PATNU` im Bash-Skript direkt gar nichts zu tun hat, denn im AWK-Programm ist `QUELL_PATNU` überhaupt nicht definiert weshalb ``$QUELL_PATNU`` eine sehr unglückliche Schreibweise für ``$0`` ist.

`$e3` sollte am Ende immer den gleichen Wert wie `$pf` haben, also kann man sich die Berechnung dafür auch sparen.

Die Berechnungen von `$quell_pfad` und `$ziel_pfad` sind bis auf die beiden Variablennamen identisch — auch in Shell-Skripten kann man Funktionen definieren statt fehleranfälliger ”Programmierung” durch kopieren und einfügen.

Die ganzen AWK-Aufrufe sind allerdings auch ziemlicher übertrieben, denn die Bash bietet genug Funktionalität um das auch ohne AWK zu lösen. Nämlich ein eingebautes ``printf`` und die Möglichkeiten die „parameter expansion” so bietet.

*Mitten* im Skript wird eine Konstante mit einem Pfadnamen definiert.

Man muss bei beiden Sicherheitsnachfragen offenbar tatsächlich entweder 'Ja' oder 'Nein' — in genau der Schreibweise eingeben, denn gibt man bei einer der beiden Abfragen etwas anderes ein, so wird fälschlicherweise ein Logeintrag geschrieben.

Setzen von überflüssigen Semikolons an Zeilenenden scheint nach dem Zufallsprinzip gemacht worden zu sein.

Bei dem Skript ist also nicht nur die API kaputt.

[BlackJack]

Nicht das ich am Wochenende zu viel Langeweile hätte oder so…

Code: Alles auswählen

#!/bin/bash
# 
# TODO Documentation.
#

readonly LOGFILE='/tmp/Protokoll.log'
readonly SERVER_PATH='/home/david/trpword/pat_nr/A'
readonly PATIENT_NUMBER_LENGTH=6


read_yes_no() {
    local prompt="${1:-Ja/Nein}"
    local answer
    
    read -p "$prompt: " answer
    [[ "$answer" == [jJ]* ]]
}


read_patient_number() {
    local prompt="$1"
    local max_length=${2:-$PATIENT_NUMBER_LENGTH}
    local result=''

    while true; do
        read -p "$prompt: " result

        if [ -z "$result" ]; then
            echo 'Die Patientennummer muss aus mindestens einer Zifffer bestehen!' >&2
        elif printf -v result "%0${max_length}d" "$result" 2>/dev/null; then
            if [ ${#result} -gt $max_length ]; then
                echo "Patientennummer ist zu lang (max. $max_length Ziffern)!" >&2
            elif [ "${result:0:1}" = '-' ]; then
                echo 'Patientennummern dürfen nicht negativ sein!' >&2
            else
                break
            fi
        else
            echo 'Patientennummern dürfen nur aus Ziffern bestehen!' >&2
        fi
    done
    echo "$result"
}


create_path() {
    local base="$1"
    local patient_number="$2"

    echo "$base/${patient_number:0:3}/${patient_number:0:4}/$patient_number"
}


log_merge() {
    local username=$1
    local old_patient_number=$2
    local new_patient_number=$3
    local merge_success=$4

    if [ ! -e "$LOGFILE" ]; then
        echo 'Protokoll' > "$LOGFILE"
    fi
    echo >> $LOGFILE
    echo "Datum $(date '+%d.%m.%y %H:%M Uhr')" >> "$LOGFILE"
    echo "alte Patientennummer: $old_patient_number" >> "$LOGFILE"
    echo "neue Patientennummer: $new_patient_number" >> "$LOGFILE"
    echo "Ausgeführt von: $username" >> "$LOGFILE"
    echo "Fehlercode (0 = ok): $merge_success" >> "$LOGFILE"
}


merge_patient_numbers() {
    local old_patient_number=$1
    local new_patient_number=$2

    dvmerge "$old_patient_number" "$new_patient_number" diffpat delpat
    cp -a \
        "$(create_path "$SERVER_PATH" "$old_patient_number")"/* \
        "$(create_path "$SERVER_PATH" "$new_patient_number")"

    true  # FIXME Merge is always successful...
}


main() {
    local username
    local old_patient_number
    local new_patient_number
    local merge_success

    echo '-----Programm zum Zusammenführen von Patienten-----'
    echo
    echo 'Bitte geben Sie für das Protokoll Ihren Namen ein:'
    read username

    echo
    old_patient_number=$(\
        read_patient_number 'Bitte geben Sie die ALTE Patientennummer ein')

    echo
    new_patient_number=$(\
        read_patient_number 'Bitte geben Sie die NEUE Patientennummer ein')

    echo
    echo "Wollen Sie die ALTE Patientennummer: $old_patient_number"
    echo "mit der NEUEN Patientennummer: $new_patient_number Zusammenführen?"
    echo
    if read_yes_no; then
        echo 'Sind sie sicher?'
        if read_yes_no; then
            merge_patient_numbers "$old_patient_number" "$new_patient_number"
            merge_success=$?
            if [ $merge_success -eq 0 ]; then
                echo
                echo '### Die Patienten wurden erfolgreich zusammengeführt ###'
                echo
            fi
            log_merge \
                "$username" \
                "$old_patient_number" \
                "$new_patient_number" \
                "$merge_success"
            echo 'Programm wird beendet!'
            sleep 5
            exit 0
        fi
    fi
    echo 'Das Programm wird aus Sicherheitsgründen beendet!'
    sleep 3
    exit 1
}

main

[Hyperion]

@BlackJack: Du weißt schon, dass Du jetzt irgend wann Ärger bekommst, wenn Dein Programm einen Fehler produziert? Denn ich wette das wird dann demnächst bei denen eingesetzt

[BlackJack]

@Hyperion: Hm, vielleicht hätte ich da absichtlich etwas fieses einbauen sollen.

[Hyperion]

@Hyperion: Hm, vielleicht hätte ich da absichtlich etwas fieses einbauen sollen.

Dann aber nur mit der "Do what you ******* want" Lizenz

``readonly`` kannte ich übrigens noch nicht! Interessant, was die Bash doch so alles kann...

[Kalli87]

@BlackJack: Danke das du dir diese Mühe gemacht hast aber ich lass lieber das alte Skript. Wenn es wirklich so schlimm is wie du es geschrieben hast dann is es halt so und ich vermute mal das Skript hat auch mein Chef geschrieben

Trotzdem danke BlackJack


Kurze Frage, ich hab eine Fehlermeldung, kann die mir mal einer erklären?

Traceback (most recent call last):
File "/shellscript.py", line 17, in <module>
out = proc.communicate('ls -l')
File "/usr/lib64/python3.4/subprocess.py", line 959, in communicate
stdout, stderr = self._communicate(input, endtime, timeout)
File "/usr/lib64/python3.4/subprocess.py", line 1601, in _communicate
input_view = memoryview(self._input)
TypeError: memoryview: str object does not have the buffer interface

[Sirius3]

@Kalli87: mit communicate kommunizierst Du mit der Außenwelt, und die versteht nur Bytes. Du mußt also Deinen String in eine Folge von Bytes umwandeln (Stichwort: encoding).

[Kalli87]

Ich hab mal bissl probiert, das kam dabei heraus:

Code: Alles auswählen

import tkinter
from tkinter import Frame, Label, NSEW, Entry, Button
import subprocess

class Application(Frame):

    def __init__(self, master):
        Frame.__init__(self, master)
        self.hauptframe()

    def hauptframe(self):
        menue_frame = Frame(root, name='haupt_frame', width=500, height=150)
        menue_frame.focus_set()
        menue_frame.grid(row=1, sticky=NSEW)

        Label(menue_frame, text="Geben Sie eine IP oder eine Host-Adresse für den Ping ein:", font=15, width=20).grid(row=1, column=1, pady=5, padx=5, sticky=NSEW)
        self.host = Entry(menue_frame, width=20, font=15)
        self.host.grid(row=1, column=2, sticky=NSEW)

        Button(menue_frame, text="Ping", width=20, command=self.shell, takefocus=1).grid(row=8, column=2, pady=5, padx=5, sticky=NSEW)

    def shell(self):
        ping = subprocess.Popen(['ping', '-c 2', self.host.get()], stdout=subprocess.PIPE).communicate()[0]
        print(ping)


root = tkinter.Tk()
root.title('ping')
app = Application(root)
root.mainloop()

Kann man die Ausgabe eigentlich noch bissl anpassen? Weil bei mir wird in der Ausgabe alles in eine Zeile geschrieben, schöner wäre es wenn er nach jedem Ping eine neue Zeile beginnt.

[Sirius3]

@Kalli87: dass Sternchenimporte, speziell bei TK, schlecht sind, sollte Dir doch in Deiner Forumskarriere hier schon mal jemand gesagt haben. Auch die Verquickung von GUI mit Geschäftslogik ist, im hinblick auf Dein eigentliches Problem, eher ungünstig. Wenn Du Parameter bei pOpen als Liste übergibst, sollte jedes Argument auch ein Element der Liste sein; dass "-c 2" funktioniert dürfte also eher zufall sein.

[Kalli87]

Ja Sirius3 wurde mir mehrmals gesagt......
Und den Rest bitte nochmal auf deutsch da ich nichts verstehe.

ps. Hab mein Import angepasst

[Hyperion]

Und den Rest bitte nochmal auf deutsch da ich nichts verstehe.

Deine (sagenhaft schlecht benannte!) Methode ``shell`` sollte *nicht* direkt das ``subprocess.Popen`` aufrufen, sondern eine Funktion (oder ggf. Methode), die *außerhalb* der GUI-Klasse definiert ist, die den passenden Aufruf zum Shell-Script realisiert. In der Methode ``shell`` (die übrigens einen wahnsinnig schlechten Namen hat, um das noch einmal zu erwähnen!) sollst Du dann nur diese ggf. mit entsprechenden Parametern aufrufen.

Ach ja... die Methode ``hauptframe`` hat ebenfalls einen ziemlich schlechten Namen...

Zur Thematik der Trennung zwischen GUI und Domänen / Geschäftslogik empfehle ich mal diesen Thread und Beitrag. Aber bitte nicht die Kommentare von Sophus als Quell der Erkenntnis nutzen, sondern die der Regulars hier

[Kalli87]

Viele Seiten zum lesen...
Was mir aber wieder aufgefallen is wie leicht meine Frage ausgewichen wurde und stattdessen wieder über alles andere geschrieben wurde

Eigentlich is das Stückchen hier ja nur ein Test-Code gewesen um ein "Bash" Befehl zu verwenden. Damit ich das Verständnis für die Übergabe an das eigentliche Skriptes besser verstehe. Aber da wurde lieber mein Code, wieder einmal, zerpflückt Die ganze Lektüre im Internet die ich gefunden habe und versuche damit etwas umzusetzen ist einfach Müll in meinen Augen bzw. schlecht erklärt. Ich bin eher der Typ der klar was vor Augen haben muss wie ein Beispiel was ähnlich aufgebaut ist aber in die Richtung geht.
Mit dem Stückchen hier hab ich gedacht ich bin auf dem richtigem weg, anscheinend hab ich mich getäuscht oder kann einer mir mal bitte sagen ob ich damit auf dem Holzweg bin?

[BlackJack]

@Kalli87: Du bist insofern auf dem Holzweg als dass der letzte Quelltext nicht mehr auf die Fragestellung beschränkt ist, die da lautet externe Programme aufzurufen. Lös *das* Problem doch erst einmal *komplett*, bevor Du da jetzt mit irgendwelchem GUI-Kram ankommst.

[Hyperion]

Eigentlich is das Stückchen hier ja nur ein Test-Code gewesen um ein "Bash" Befehl zu verwenden. Damit ich das Verständnis für die Übergabe an das eigentliche Skriptes besser verstehe. Aber da wurde lieber mein Code, wieder einmal, zerpflückt

Dieses Argument mit "Testcode" kann ich nicht mehr hören... ständig erklären uns Anfänger, dass sie schlechte Namen ja nur wählen, weil das Testcode ist usw. bla bla bla... zeigt man uns dann aber mal echten Code, sieht das alles *genauso* aus! Imho klar, weil es *ohne* Übung nicht besser wird und genau die holt man sich *auch* und *insbesondere* beim Schreiben kleiner Schnippsel.

Also: Versuche doch auch solch ein Testscript so sauber wie Produktivcode zu implementieren. Was glaubst Du, wieso Code von BlackJack und anderen hier immer so sauber aussieht? Und das ist für uns Regulars hier fast immer *Testcode* bzw. Spaß-Code...

Denk da mal drüber nach!

Zu Deiner Formatierungsfrage: Wie sieht es denn aus und wie *soll* es denn aussehen? Das würde schon Klarheit bringen.

Ein kleines Projekt zum Testen ist übrigens schon mal gut. Ob man das mit GUI haben muss, weiß ich an dieser Stelle nicht, aber ok.

[Kalli87]

Code: Alles auswählen

import subprocess

proc = subprocess.Popen(['bash', '/DVMERGE/DVMERGE.sh'])
proc.communicate()

@Hyperion: Was is an diesem Code falsch? (Wenn überhaupt etwas falsch ist)
Ich mein, eingebunden ist er und wird auch gestartet, was nur auffällt ist das die GUI dabei hängen bleibt bis das Script durchgelaufen ist.
Das soll jetzt nur rein informativ für mich sein!

Gut beim nächsten "Testcode" werde ich drauf achten das er "sauber" ist.

Zur Formatierungsfrage naja sieht halt so aus und alles in einer Zeile

Code: Alles auswählen

b'PING www.google.de (173.194.116.127) 56(84) bytes of data.\n64 bytes from fra02s27-in-f31.1e100.net (173.194.116.127): icmp_seq=1 ttl=56 time=18.6 ms\n64 bytes from fra02s27-in-f31.1e100.net (173.194.116.127): icmp_seq=2 ttl=56 time=19.0 ms\n\n--- www.google.de ping statistics ---\n2 packets transmitted, 2 received, 0% packet loss, time 1001ms\nrtt min/avg/max/mdev = 18.666/18.836/19.007/0.218 ms\n'

Ich werd einfach was schreiben ohne GUI und und binde das Script ein und starte es aber verfehlt das nicht bisschen den Sinn?
Das Script selbst fragt ja nach die Eingabe ab und wenn das "nur" in der Konsole läuft funktioniert es ja oder seh ich das gerade verkehrt?