-
Notifications
You must be signed in to change notification settings - Fork 2
Orderunterstützung für Veranstaltungsauswahl, Integration von myHAW Scraper #177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fad3c10 to
c3598b9
Compare
|
@EdJoPaTo ich würde mich über ein Review freuen 😃. lg |
|
Ja… Ich würde mich auch über ein Review machen freuen 😬 Zeit und zu viele Dinge… Ich versuche es erstmal mit den generellen Gedanken und hoffe, dass ich immer mal Zeit hab, um reinzuschauen. Auf jeden Fall cool, dass du da rein schaust und das Projekt an die neuen Herausforderungen anpassen magst! Da recht viel angefasst wird, würde ich wohl versuchen sowas wie die Mensa doch aus diesem PR herauszuziehen ums einfacher zu halten. (Hab ewig nicht mehr in den Code geschaut, der Fix wirkt etwas komisch, kann aber auch einfach daran liegen, dass der Code schon ein paar Tage alt ist und ich den länger nicht gesehen hab. Von daher ist unabhängig darüber zu diskutieren vielleicht auch ganz gut.) Und ich überlege ein bisschen, ob die changes als Feature vielleicht einfach komplett verschwinden sollten. Es wird nur wenig genutzt, ist ein wenig fehleranfällig gebaut (könnte mal verbessert werden) und die wenigen, die es nutzen haben fast ausschließlich Termine entfernt, sonst nichts. Das Feature weiterzutragen und zu schauen, dass es mit den neuen Anforderungen weiter sinnvoll funktioniert, ist glaube ich den Aufwand nicht wert, auch wenn es irgendwie cool ist. (Side note: wenns gemerged wird, wird es ein squash and merge, du brauchst also keine rebases machen, sondern kannst gerne einfach weiter drauf mergen oder merge Commits machen, das landet nachher sowieso alles in einem einzelnen Commit und die Entwicklung mit einzelnen Commits im PR ist, finde ich, auch etwas leichter nachzuvollziehen.) |
Vielen Dank, dass du dir trotzdem Zeit genommen hast 👍
So viel ist das gar nicht. Wie gesagt ist das meiste nur die Änderung vom Event-Namen auf die Event-Id, also nur eine Umbenennung des Feldes. "Richtige Änderungen" sind nur in zwei Dateien:
Ok, ich erkläre trotzdem mal kurz, warum die zwei Zeilen das Problem beheben: Wenn man eine neue Session hat (also entweder wenn man neuer Nutzer ist oder wenn der Bot neu gestartet wurde) ist das
Diesen Teil deiner Antwort verstehe ich nicht ganz. Von welchem Feature redest du? Dem Kalenderteil des Bots? Ist das nicht der hauptsächliche Sinn des Ganzen?
Ich kann da nur für mich sprechen und ich habe den Bot genau so genutzt. Also nur um Termine in den Kalender zu übernehmen und manchmal ausfallende Veranstaltungen entfernen. (Und natürlich den Mensateil.) Das Ganze fand ich sehr praktisch und würde es schade finden wenn es nicht mehr geht. Deswegen hab ich mir ja auch die Mühe gemacht den Scraper zu schreiben und die Änderungen hier zu machen. Die anderen Funktionen habe ich nicht genutzt, da ich sie nicht brauche, wobei die zumindest jetzt bei den Änderungen auch keine Anpassungen brauchten. |
Mir ging es eher darum, dass das ein getrenntes Problem ist und dass der PR übersichtlicher wird, wenn unterschiedliche Probleme in unterschiedlichen PRs besprochen werden, die dann auch unterschiedlich leicht/schnell gemerged werden können. Sorry, wenn ich mich da verwirrend ausgedrückt hab. 😇
Das Zuordnen der Changes zu den Events ist… historisch gewachsen und fehleranfällig und wenn sich jetzt IDs und Pfade ändern, hab ich den Verdacht, dass das nicht unbedingt besser wird. (Und muss auch etwas umständlich entfernt werden, weils getrennte Dinge sind, Grobe Idee einer user-config, die weniger fehleranfällig wäre (und wohl unabhängig von diesem PR): {
"events": {
"someEventIdOrOtherKindOfUniqueReference": {
"removed": [
"2042-12-24T13:37:00"
]
}
}
}To the rest… soon™ when time 😇 |
Ok, jetzt hab ich verstanden was du meinst 😅. Da stimme ich dir zu. Ich kann mir das gerne mal ansehen. |
9b465ff to
00d1909
Compare
|
Ich hab hoffentlich jetzt morgen / am Wochenende etwas Zeit mir das anzuschauen. Ich wollte auf jeden Fall noch mal nen Release vom Downloader und Parser im aktuellen Zustand machen, damit es danach breaking changes geben kann. Ich überlege, ob ich die Changes, abgesehen vom remove von einzelnen Terminen, rauswerfe. Dann sollte sowohl parser als auch Telegram Bot deutlich einfacher werden. Was auch noch eine spannende Frage wird, wie denn dein downloader projekt dann mit dem bei mir laufenden Bot funktionieren wird, aber solange das nen Container ist sollte das ja gehen? Noch andere Gedanken? |
Also ich hab bisher auch (bis auf ein mal, wo ich die Zeiten angepasst hab) nur das "Termin entfernen" benutzt. Von dem her habe ich zumindest kein Problem damit.
Ich habe für mich und ein paar Kommilitonen eine Testinstanz von meiner Version aufgesetzt und da läuft der Scraper einfach in einem Container, der ein geteiltes Volume mit dem Bot und dem Parser hat. Im Moment hab ich es so eingestellt, dass der Scraper alle 6 Stunden läuft (insbesondere auch, weil ein Durchgang ca. 12-18 min. braucht). Man könnte das dann ähnlich machen wie bei dir jetzt und den Scraper das nach einem Zeitplan in ein git-Repo zu pushen lassen, wo sich der Bot/Parser die Dateien dann besorgt. |
|
Ach ja... Um die Änderungen hier zu testen ist es sicherlich praktisch einen Satz von den Event-Dateien zu haben. Hier ein Ordner mit den Event-Dateien + |
|
Die eventfiles in ein Git Repo zu schieben klingt gar nicht so schlecht. Auch mit dem Änderungs-History Gedanken.
Hab bei meinem Downloader auch irgendwie 5 Sekunden oder so zwischen zwei requests, um harmlos für den Server zu sein. Dadurch braucht(e) der auch immer ne weile. |
hm… sind die IDs überhaupt stabil…? Wie häufig ändern die sich potenziell? |
Wenn ich mir die Einträge anschaue, frage ich mich, neben den IDs, ob die überhaupt langfristig stabil sind, was der Unterschied zwischen Name und Originalem Titel ist? {
"Id":"25435_5",
"Name":"M 6.1.5 (Selbstkompetenz)",
"Location":"2.08, Alexanderstraße 1",
"Description":"Dozent: Stefanie Ehlers\nParallelgruppe: 5\nMax. Anzahl von Teilnehmer/-innen: 22\nRhythmus: Einzeltermin\n\nOriginaler Titel: BABE M 6.1.5 Selbstkompetenz (5. Parallelgruppe)",
"StartTime":"2025-12-13T09:00:00",
"EndTime":"2025-12-13T17:00:00"
}, |
|
Generell der Teil von der ID vor dem '_' ist der, den myHAW dem Modulteil (also Vorlesung, Praktikum ,...) gibt. In einem Modulteil kann es dann mehrere Parallelgruppen geben. Wenn es mehrere gibt haben diese i.d.R. jeweils eine Nummer. Der zweite Teil der ID ist dann genau diese Nummer. Wenn es nur eine Parallelguppe gibt ist der 2. Teil immer Mit diesem Verfahren sollten die ID für Vorlesungen sehr stabil und für Veranstaltungen mit mehreren Parallelgruppen solange stabil sein, wie die Parallelgruppennummern nicht geändert bzw. für neue Gruppen wiederverwendet werden. |
Originaler Name ist der komplette Name der Parallelgruppe von myHAW. Leider ist der häufig nicht besonders gebrauchbar und hat ein sehr sehr inkonsistentes Format, da das ein Freitextfeld ist. Name ist dann das Feld, wo die Inkonsistenzen möglichst gut beseitigt werden. Manchmal gibt es aber noch Informationen, die der Scraper nicht lesen kann. Für den Fall ist dann das Feld Originaler Titel als Fallback. |
Wenn der Parser nicht angefasst werden muss, der ja auf den Namen arbeitet, müsste die eventId doch auch relativ egal im Telegram Bot sein? Wenn der downloader und Dateinamen die nutzen, müsste das ja ok sein, aber innerhalb der Dateien ist der glaube ich dann eher nicht relevant? Oder übersehe ich noch was? Was ist der Nachteil bei den Namen zu bleiben? Ändern sich die potenziell zu häufig? |
|
Da ich aktuell erstmal alles mit changes rausgeworfen hab aus dem Telegram Bot gibts relativ viele merge conflicts… Der Parser ist jetzt angepasst und könnte mit den changes umgehen, von daher sollte ich da vielleicht (hoffentlich morgen) noch mal ran un die changes im Bot doch wieder drin behalten, nur eben an nem anderen Ort in der Userconfig. Sorry für das durcheinander was dadurch entsteht 😬 |
Also meinst du, dass man das Feld
Einerseits das und zusätzlich gibt es viele Doppelbelegungen, wodurch das unmöglich wird bei den Namen zu bleiben. Zudem sehe ich da keinen großen Vorteil, bis auf die menschliche Lesbarkeit in der Userconfig. Wie schon gesagt die Änderungen in Bot und Parser sind deswegen recht minimal, da die beiden der Inhalt des Namen bzw. der Ids eigentlich nicht interessiert. Es ist nur wichtig, das damit die entsprechende Datei gefunden werden kann. Und im Falle des Bots muss nun noch der Name in den EventDetails gespeichert werden, damit man den nicht immer aus dem Directory/den Event-Dateien lesen muss bzw. damit wenn es das Event nicht mehr gibt man trotzdem noch den Namen angezeigt bekommen kann. |
Das müsste dann spätestens hier auf die Nase fallen: Und der Name kommt aus der Userconfig, wo der vom TelegramBot gesetzt wird. |
|
Nein, das ist kein Problem. Ich versuche das nochmal zu erklären: Angenommen der Parser hat bereits eine Userconfig eingelesen1 . Wenn dann die Funktion Diese Funktion ruft dann wiederum für das entsprechende Event die Es wäre natürlich sinnvoll die variablen von In meiner Test-Instanz funktioniert das auch alles gut 😇. Footnotes
|
Was den Namen vorher Eindeutigkeit gebracht hat, war der jeweilige Studiengang / Semester im Namen als Prefix. Wenn das irgendwie in den Namen kommen könnte, wäre der Name wieder eindeutig? Hab teilweise gesehen, teilweise scheint das in den originalen Titeln drinzustecken, aber nicht überall? Bei besagtem GSP zum Beispiel nicht… In der directory.json steckt zumindest die Information mit drin, allerdings ausgeschrieben als Tree, nicht als Kürzel? Dafür hat die ID aber auch den Vorteil, keinerlei Zeichen unpassend für unterschiedliche Dateisysteme zu beinhalten. Das directory ist auch etwas nervig mit unterschiedlichen Tiefen. Hab schon überlegt, ob eine einfachere Datenstruktur hilfreich wär: {
"007_1": [
"Department Foo",
"Bachelor Bar",
"42. Semester",
"Grundlagen Wirrwarr Praktikum",
"GWP/01"
],
"007_1": [
"Department Foo",
"Bachelor Bar",
"42. Semester",
"Grundlagen Wirrwarr",
"Grundlagen Wirrwarr Praktikum",
"GWP/02"
],
}Und dann lässt sich der Name aus der ID generieren: Und gleichzeitig kann nen Filter bis zu depth 2 alles für den Bachelor nehmen. Das schafft zwar recht viel inhaltliche Wiederholung, macht aber die Datenstruktur sehr flach. |
Dann können die anderen Event Details / Changes nicht mehr funktionieren, wie sie aktuell funktionieren und müssen im Parser umgebaut werden. Die beziehen sich nämlich auf die Namen, nicht die IDs. |
Da habe ich tatsächlich übersehen. Wenn das Feld |
Leider gibt es in myHAW keine Kürzel für die Studiengänge. Zudem sind die Modulkürzel auch alles andere als Konsistent. Der Scraper versucht allerdings möglichst gut die Inkonsistenzen zu beseitigen.
Das kann ich nicht genau sagen, da die Leute, die das da eintragen teilweise sehr "kreativ" sind und sich auch nicht unbedingt untereinander absprechen.
😄 das ist leider dem geschuldet, dass das ein Freitextfeld ist, wo die alles mögliche reinschreiben können.
Die Baumstruktur kommt ziemlich direkt von dieser Seite aus myHAW: https://myhaw.haw-hamburg.de/qisserver/pages/cm/exa/coursecatalog/showCourseCatalog.xhtml?_flowId=showCourseCatalog-flow . Je länger man sich die ansieht, desto mehr Inkonsistenzen kann man da finden, was das Parsen/Vereinfachen nicht unbedingt simpler macht...
Würde ich auch sagen. Deswegen konnte ich auch die replace-Operationen hier im Code entfernen.
Vom Code her muss man dadurch auf Rekursion/Stacks zurückgreifen. Das macht es etwas komplexer. Wobei die gesamte Logik, die das im Bot verarbeitet nur knapp über 100 Zeilen lang ist (siehe Datei
Diese Struktur verstehe ich nicht ganz. Ich gehe mal davon aus, dass die Liste jeweils die Unterordner beinhaltet? Für mich sieht das sehr umständlich aus das dann darzustellen.
Sollen die Events dann überhaupt als Ordner dargestellt werden? Und ist der Name dann nicht etwas lang?
Auch hier weiß ich nicht genau was du meinst. Mit meiner rekursiven Implementierung kann man auch ab einer gewissen Tiefe filtern.
Welchen Vorteil hat denn eine flache Datenstruktur? Generell verstehe ich nicht ganz, warum du so an dem Namen als Schlüssel festhalten möchtest. Die Ids sind garantiert eindeutig, für Vorlesungen garantiert stabil und für Praktika so stabil wie es geht. Zudem müssten auch über Semester hinweg gleich bleiben. |
|
Was mir gerade auch noch einfällt: Es gibt Veranstaltungen, die an verschiedenen Stellen im Baum auftauchen. Gerne sind das Wahlpflichtmodule, die von mehreren Studiengängen geteilt werden. Es gibt aber auch noch weitere Module, bzw. manchmal auch nur Teile von Modulen, wo das auch so ist. In der |
Das sollte das jetzt möglich machen: HAWHHCalendarBot/parser@1fedeab |
|
Ich glaube vor allem für parser und co sind die extra unterordner relativ nervig. Der einzelne Unterordner für die ganzen Dateien ist vermutlich noch ganz gut, damit der Hauptordner übersichtlich bleibt. |
move the simple path to the top as early abort
search pattern inside path, so path is the more stable one
The variable name does not provide any benefit
EdJoPaTo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Habs jetzt mal alles getestet, läuft alles ✨
Parser läuft auch mit dem eventfiles Repository und den userconfigs.
Hab noch ein paar Dinge angepasst, wenn du also noch Gedanken, Bugs oder weirde Änderungen dazu hast, gerne her damit! Ansonsten, noch was vor nem Merge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Danke für deine Änderungen👍. Bei vielem frage ich mich, warum ich da nicht selbst drauf gekommen bin 😅.
Ich hab noch eine Frage/Vorschlag (s.u.)
Ansonsten hätte ich auch nichts mehr offen 👍
Ein anderes Paar Augen sieht andere Dinge, die dann offensichtlich werden. Thats always the case. Und vieles hätte ich vermutlich nicht gesehen, wenn du es nicht angestoßen hättest 😇 Danke für deinen Pull Request und sorry, dass es sich ja doch mehr gezogen hat, aber dafür wirkt es jetzt echt gut! (Ich merge und aktualisiere mal heute Abend.) |
Having let directory and const DIRECTORY was weird. Writing the directory name into the paths which are only used in exactly one module is relatively fine.
|
🚀 |
|
Das war intensiv 🥇 |
Hallo,
hier ist der aus #176 versprochene PR für die Integration von den Daten vom myHAW Scraper in den Bot.
Hier eine Übersicht über die Änderungen:
Events werden jetzt unter ihrer ID aus myHAW gespeichert (Format:
"${id}_${Gruppennummer}"), damit die Zuordnung besser Funktioniert, da die Kurznamen leider nicht unbedingt eindeutig über alle Studiengänge sind. Der Name des Events ist jetzt in den EventDetails gespeichert. Dadurch, dass sich eigentlich nur der Name der Events im Vergleich zu Vorher ändert, muss relativ wenig Code geändert werden. Die meisten Änderungen sind die Umbenennungen vonnamezueventId. Zusätzlich muss der Parser dadurch auch nur kaum angepasst werden (-> PR: refactor!: remove timezone info from event entries parser#44).Unterstützung für Ordner bei der Veranstaltungssuche: Wenn man Veranstaltungen hinzufügt werden diese jetzt in Ordnern dargestellt. Da so deutlich weniger Einträge pro Seite dargestellt werden müssen, habe ich die Anzahl der Spalten auf 1 verringert, um die Übersichtlichkeit zu verbessern. Hier eine kleine Demo:
bot_with_dirs.webm
Die Filterfunktion unterstützt auch die Ordnerfunktion. Ordner werden zwar in den Suchergebnissen nicht mehr dargestellt, aber es wird nur ab dem Ordner gesucht in dem man gerade ist.
Zusätzlich ist mir noch ein Bug bei der Mensaauswahl aufgefallen, wenn man eine "frische" Session hat. (Details: siehe Commitnachricht)