Verstehe nicht warum

[bakedstroh]

Hallo Community,
hab hier einen Code bei dem ich etwas nicht verstehe...
Sinn dahinter: Das Programm läuft auf einem Raspberry und schickt mir jeden Tag aktuelle Corona-Zahlen (Sinnhaftigkeit dahinter wird bitte nicht diskutiert ).
Mein Problem: Statt 1ner E-Mail bekomme ich tweilweise 50+, und manchmal funktioniert es und es wird nur eine E-Mail versendet. Ich verstehe nicht warum... vermute mal das das Problem bei dem "schedule teil" liegt möglicherweise...
Zu einfach schlecht programmierten zeilen bitte ich euch auch um Kommentare

Code: Alles auswählen

import requests
import smtplib
import schedule
from bs4 import BeautifulSoup
from time import sleep


#daten
gmail_user = "email adresse zum einloggen"
gmail_user_pwd = "passwort zum einloggen"
url = "https://bmi.gv.at/news.aspx?id=4A7171477A51625143334D3D"
subject = "Corona Info"



def banner():
    banner = "###############################\n" \
             "----Corona_Info_MailService----\n" \
             "###############################"
    return print(banner)

def requests_get_info(url):
    req = requests.get(url)
    bfs_Object = BeautifulSoup(req.text, "html.parser")
    daten_raw = str(bfs_Object.find_all("strong"))
    daten_replace1 = daten_raw.replace("[<strong>Corona-Virus</strong>, <strong>", "")
    daten_ready_replaced = daten_replace1.replace("</strong>]", "")
    daten = daten_ready_replaced.split(", ")
    return daten

def send_mail_to(subject, corona_daten, gmail_user, gmail_user_pwd):
    #liste aller empfänger
    # weitere email adressen hier hinzufügen:               <<<===
    addressee_list = ["email@absender.foo"]
    #verbindung zu smtp server herstellen und einloggen
    smtp_server = smtplib.SMTP("smtp.gmail.com", 587)
    smtp_server.ehlo()
    smtp_server.starttls()
    smtp_server.login(gmail_user, gmail_user_pwd)

    #gestaltung der email
    for people in addressee_list:
        header = "To:" + people + "\n" + "From: " + gmail_user + "\n" + "Subject:" + subject + "\n"
        msg_scheduling = corona_daten[0] + "\n\n" + corona_daten[1] + "\n\n" + corona_daten[2] + "\n"
        message = header + msg_scheduling
        message_encoded = message.encode("iso-8859-15", errors="ignore")
        smtp_server.sendmail(gmail_user, people, message_encoded)
    smtp_server.close()
    return print("Done!")

def main():
    banner()
    daten = requests_get_info(url)
    send_mail_to(subject, daten, gmail_user, gmail_user_pwd)

if __name__ == '__main__':
    which_time = str(input("Zu welchen Zeit erwarten Sie die Email?   [00:00]\n"))
    print("[RUNNING] corona_service is running...")
    while True:
        schedule.every().day.at(which_time).do(main)
        schedule.run_pending()
        sleep(60)

[nezzcarth]

Wenn ich das richtig vestehe, erstellst du alle 60 Sekunden einen neuen Job; das führt dann natürlich zu vielen Dubletten. So oder so würde ich das Scheduling aber auch gar nicht in Python machen. Üblicherweise nimmt man für so etwas unter Linux Cron-Jobs oder Systemd-Timer-Units, also Funktionalitäten, die das Betriebssystem selbst bereitstellt. Dein Skript sollte m.M.n. nur die Daten abholen und die Mail versenden, mehr nicht.

Den restlichen Code habe ich nicht wirklich angesehen, aber die Zeilen, in denen du "return print ..." stehen hast, sollte man überarbeiten (das return entfernen).

[bakedstroh]

Hatte ich schon gehört, werde es gleich direkt mit Cronjobs versuchen.
Danke fürs Feedback

[__blackjack__]

@bakedstroh: Anmerkungen zum Quelltext:

Konstanten werden in Python KOMPLETT_GROSS geschrieben.

Der Code im ``if __name__ == "__main__":``-Zweig gehört in eine Funktion, die üblicherweise `main()` heisst. Das heisst die vorhandene `main()` muss anders heissen.

`input()` liefert bereits eine Zeichenkette, es macht keinen Sinn mit der `str()` aufzurufen.

`banner()` ist kein guter Name für eine Funktion. Funktionen (und Methoden) werden üblicherweise nach der Tätigkeit benannt die sie durchführen. Dann weiss der leser was die machen und kann die Namen von Funktion und Methoden auch leichter von eher passiven Werten unterscheiden. `print_banner()` wäre beispielsweise passender, aber wenn man sich den Inhalt der Funktion anschaut ist die Frage ob dieser eine `print()`-Aufruf tatsächlich in eine eigene Funktion muss.

`req` ist als Name falsch, denn das ist kein `request` sondern eine `response`. Kryptische Abkürzungen sollte man auch vermeiden.

Man sollte den Fall berücksichtigen, dass der Status der Antwort nicht OK ist.

`*_Object` als Namenszusatz ist nicht sinnvoll weil das keinerlei Information beinhaltet, denn *alles* was man an einen Namen binden kann ist ein Objekt. Da könnte/müsste man dann konsequenterweise an jeden Namen `_Object` anhängen. Dann wird aber auch klar wie sinnfrei das ist. Bei `bfs` gilt dann wieder: keine Kryptischen Abkürzungen.

Der Rest der Funktion wird dann gruselig weil Du auf der Zeichenkettendarstellung einer Liste mit `bs4.PageElement`-Objekten mit Zeichenkettenoperationen herumpopelst statt wie vorgesehen mit den Objekten zu arbeiten. Man macht so ziemlich grundsätzlich nichts an einer Zeichenkettendarstellung einer Liste. Das ist immer gruselig falsch. Zeichenkettendarstellung von Listen sind zur Ausgabe bei der Fehlersuche, nicht um die irgendwie weiterzuverarbeiten.

Einfach alle <strong>-Tags in der gesamten Seite zu suchen ist auch nicht besonders robust. Man sollte sich da mehr an der Seitenstruktur orientieren. Beispielsweise im ersten <article> das erste <div> mit einem <p> mit einem <strong> finden. Und da dann denn Text nehmen, was genau nur das ist was Du haben willst.

`SMTP`-Objekte sind Kontextmanager die man mit ``with`` verwenden kann und sollte.

Grunddatentypen haben in Namen nichts zu suchen. Man ändert den Typen gerne mal während der Programmentwicklung und dann hat man entweder falsche, irreführende Namen im Quelltext stehen, oder man muss dann auch immer alle betroffenen Namen ändern.

Die Empfänger würde ich nicht in einer Funktion verstecken sondern am Anfang als Konstante definieren.

Bitte einmal im Wörterbuch nachschlagen was `people` bedeutet. Sofern Das da nicht bei der UN läuft sind die einzelnen Empfänger der Mails sicher keine `people`.

Zeichenketten und Werte mit ``+`` zusammenstückeln ist eher BASIC denn Python. In Python gibt es dafür Zeichenkettenformatierung mit der `format()`-Methode und f-Zeichenkettenliterale.

Die "Done!"-Ausgabe gehört eher eine Funktion höher in die Aufrufhierarchie, weil dort bereits Benutzerinteraktion via `print()` stattfindet.

Zwischenstand (ungetestet):

Code: Alles auswählen

#!/usr/bin/env python3
import smtplib
from time import sleep

import requests
import schedule
from bs4 import BeautifulSoup

SMTP_USER = "email adresse zum einloggen"
SMTP_USER_PASSWORD = "passwort zum einloggen"
URL = "https://bmi.gv.at/news.aspx?id=4A7171477A51625143334D3D"
SUBJECT = "Corona Info"
RECIPIENTS = ["user@mail.foo"]


def get_info(url):
    response = requests.get(url)
    response.raise_for_status()
    return (
        BeautifulSoup(response.text, "html.parser")
        .select_one("article div p strong")
        .text.split(", ")
    )


def send_mail(recipients, subject, corona_daten, user, password):
    with smtplib.SMTP("smtp.gmail.com", 587) as smtp_server:
        smtp_server.ehlo()
        smtp_server.starttls()
        smtp_server.login(user, password)
        for recipient in recipients:
            body = "\n\n".join(corona_daten)
            smtp_server.sendmail(
                user,
                recipient,
                (
                    f"From: {user}\n"
                    f"To: {recipient}\n"
                    f"Subject: {subject}\n"
                    f"\n{body}\n"
                ).encode("iso-8859-15", errors="ignore"),
            )


def do_job():
    print(
        "###############################\n"
        "----Corona_Info_MailService----\n"
        "###############################"
    )
    send_mail(
        RECIPIENTS, SUBJECT, get_info(URL), SMTP_USER, SMTP_USER_PASSWORD
    )
    print("Done!")


def main():
    time_text = input("Zu welchen Zeit erwarten Sie die Email?  [00:00]\n")
    print("[RUNNING] corona_service is running...")
    schedule.every().day.at(time_text).do(do_job)
    while True:
        schedule.run_pending()
        sleep(30)


if __name__ == "__main__":
    main()

[bakedstroh]

@ __blackjack__ Dankeschön für die sehr ausführliche Antwort, werde mir das nochmals ansehen und gegebene "Fehler" ändern!