for review #1

Merged
shuber merged 54 commits from init into main 2026-08-21 06:01:41 +00:00
Member
@shuber
Author
Member

die letzten commits waren nur änderungen an der website

die letzten commits waren nur änderungen an der website
.cursorrules Outdated
@ -0,0 +1,117 @@
# Ninja-TANSS Comparison Project Rules
Owner

wieso haben wir .cursorrules? ist das von opencode?

wieso haben wir .cursorrules? ist das von opencode?
Author
Member

Das habe ich hinzugefügt, habe gehofft, dass es die KI Antworten verbessert.
War aber nicht so, das entferne ich.

Das habe ich hinzugefügt, habe gehofft, dass es die KI Antworten verbessert. War aber nicht so, das entferne ich.
dgoetz marked this conversation as resolved
@ -0,0 +11,4 @@
# TANSS Login Configuration (for cookie-based authentication)
TANSS_LOGIN_USERNAME=your_username_here
TANSS_LOGIN_PASSWORD=your_password_here
Owner

sehr ordentlich, keine passwörter gepushed, perfekt!

sehr ordentlich, keine passwörter gepushed, perfekt!
Author
Member

Danke!

Danke!
dgoetz marked this conversation as resolved
@ -0,0 +1,9 @@
# Secrets
Owner

die bruno-sachen könnten wir in ein separates repo auslagern, dann ist das bisschen gesammelt und wir könnten eine große collection aufbauen, die wir versionieren

die bruno-sachen könnten wir in ein separates repo auslagern, dann ist das bisschen gesammelt und wir könnten eine große collection aufbauen, die wir versionieren
Author
Member

Wenns für dich i.O. ist, würde ich das gegen Ende des Projekts machen. So wäre es für mich einfacher, wenn ich neue Endpunkte hinzufügen muss.

Wenns für dich i.O. ist, würde ich das gegen Ende des Projekts machen. So wäre es für mich einfacher, wenn ich neue Endpunkte hinzufügen muss.
Owner

klar, passt!

klar, passt!
dgoetz marked this conversation as resolved
@ -0,0 +6,4 @@
environment:
POSTGRES_USER: admin
POSTGRES_PASSWORD: secret
POSTGRES_DB: syncninja
Owner

vielleicht könnten wir hier gleich noch deinen code als docker-image verschiffen. dann können wir das deployment über CI/CD automatisieren

vielleicht könnten wir hier gleich noch deinen code als docker-image verschiffen. dann können wir das deployment über CI/CD automatisieren
Author
Member

Ja, das wäre auch mein Plan.
Wenn ich mit der Website fertig bin, würde ich das in Angriff nehmen.

Ja, das wäre auch mein Plan. Wenn ich mit der Website fertig bin, würde ich das in Angriff nehmen.
Owner

passt!

passt!
dgoetz marked this conversation as resolved
@ -0,0 +1,92 @@
"""CLI and API entry point for the Ninja-TANSS comparison tool."""
Owner

wär cool. wenn du immer noch den author (also deinen namen dgoetz) reinschreiben würdest.
bei java gibt es da spezielle "marker" ausi (@author), vielleicht hat python sowas auch. wenn mehrere da rumpfuschen, weiß man wen man ansprechen muss bei unklarheiten

wär cool. wenn du immer noch den author (also deinen namen dgoetz) reinschreiben würdest. bei java gibt es da spezielle "marker" ausi (@author), vielleicht hat python sowas auch. wenn mehrere da rumpfuschen, weiß man wen man ansprechen muss bei unklarheiten
dgoetz marked this conversation as resolved
pyproject.toml Outdated
@ -0,0 +1,30 @@
[project]
name = "ninja-tanss-comparison"
Owner

das noch anpassen bitte (und beschreibung natürlich), ansonsten tippitoppi

das noch anpassen bitte (und beschreibung natürlich), ansonsten tippitoppi
dgoetz marked this conversation as resolved
@ -0,0 +12,4 @@
from fastapi.responses import HTMLResponse, JSONResponse
from pydantic import BaseModel, Field
from src.db.postgresql import HiddenDevice, create_tables, get_session
Owner

das modul postgresql könnten wir vllt umbennen. produktname gefällt mir nicht soo gut

das modul postgresql könnten wir vllt umbennen. produktname gefällt mir nicht soo gut
Author
Member

Wie findest du:
from src.db.postgresql => from src.db.connection

Wie findest du: from src.db.postgresql => from src.db.connection
dgoetz marked this conversation as resolved
@ -0,0 +77,4 @@
_pipeline_state.running = False
@router.post("/api/run-pipeline")
Owner

versionierung würde hier noch sinn machen. also /api/v1/run-pipeline

möchte man etwas ändern aber was altes nicht zerschießen, kann man über api-versionierung machen

EDIT: vllt kann man auch nur .../pipeline/ schreiben statt run-pipeline, das ist dann bisschen cleaner

versionierung würde hier noch sinn machen. also /api/v1/run-pipeline möchte man etwas ändern aber was altes nicht zerschießen, kann man über api-versionierung machen EDIT: vllt kann man auch nur .../pipeline/ schreiben statt run-pipeline, das ist dann bisschen cleaner
Author
Member

Also dann /api/v1/pipeline ?

Also dann /api/v1/pipeline ?
dgoetz marked this conversation as resolved
@ -0,0 +94,4 @@
return JSONResponse(status_code=200, content={"status": "started"})
@router.websocket("/api/pipeline-ws")
Owner

man könnte hier vielleicht das ws vorne in den pfad einbauen falls man mehrere ws-anwendungsfälle hat, also /api/v1/ws/pipeline oder so

man könnte hier vielleicht das ws vorne in den pfad einbauen falls man mehrere ws-anwendungsfälle hat, also /api/v1/ws/pipeline oder so
dgoetz marked this conversation as resolved
@ -0,0 +114,4 @@
pass
class EntityEntry(BaseModel):
Owner

das gehört sich glaub ich nicht in die routes.py, oder?

das gehört sich glaub ich nicht in die routes.py, oder?
dgoetz marked this conversation as resolved
@ -0,0 +125,4 @@
tanss_contract: TanssContract | None = None
class InvalidStateEntry(BaseModel):
Owner

gleiches hier

gleiches hier
dgoetz marked this conversation as resolved
@ -0,0 +133,4 @@
entities: list[EntityEntry] = Field(..., description="List of related entities")
class InvalidStatesResponse(BaseModel):
Owner

gleiches hier

gleiches hier
dgoetz marked this conversation as resolved
@ -0,0 +150,4 @@
)
@router.get(
Owner

wollen wir das vielleicht zu /states/invalid oder so ändern? dann würden wir diesen bindestrich wegbekommen

wollen wir das vielleicht zu /states/invalid oder so ändern? dann würden wir diesen bindestrich wegbekommen
dgoetz marked this conversation as resolved
@ -0,0 +165,4 @@
return store.invalid_states
class SystemMetadata(BaseModel):
Owner

das müsste alle wsl ins db-modul, sind ja datenbanktabellen

das müsste alle wsl ins db-modul, sind ja datenbanktabellen
dgoetz marked this conversation as resolved
@ -0,0 +61,4 @@
return []
def saveCache(filename: str, data: list) -> None:
Owner

das ist obsolet, wir haben inzwischen ja eine datenbakn, doer?

das ist obsolet, wir haben inzwischen ja eine datenbakn, doer?
dgoetz marked this conversation as resolved
@ -0,0 +31,4 @@
Base.metadata.create_all(bind=engine)
def get_session():
Owner

könnten wir zu singleton umbauen, dann kannst du jedes mal getSession() aufrufen und musst dir das Objekt nicht speichern. Dann haben wir eine Session über den Scope der ganzen Applikaiton

könnten wir zu singleton umbauen, dann kannst du jedes mal getSession() aufrufen und musst dir das Objekt nicht speichern. Dann haben wir eine Session über den Scope der ganzen Applikaiton
dgoetz marked this conversation as resolved
src/helper.py Outdated
@ -0,0 +5,4 @@
from .logger import logger
def printFetching(message: str) -> None:
Owner

das versteh ich nicht, wieso rufen wir nicht direkt den logger auf?

das versteh ich nicht, wieso rufen wir nicht direkt den logger auf?
Author
Member

Danke, das ist ein Restbestand, der kommt weg. Ist mir nicht aufgefallen.

Danke, das ist ein Restbestand, der kommt weg. Ist mir nicht aufgefallen.
dgoetz marked this conversation as resolved
@ -0,0 +14,4 @@
logger.info(message)
def getNested(data: dict, dot_path: str, default=None):
Owner

vielleicht kann man sich mal überlegen, ob man sowas auslagert in ein tanss-modul. vllt könnten wir uns da auch mal was mit ki basteln, um die tanss-api mit python zu bedienen, so eine standard-library. das könnte für odoo auch gut werden.

aber für jetzt: vielleicht in ein tanss/utilities modul oder so, dann kann man das wiederverwenden für zukünftige projekte

vielleicht kann man sich mal überlegen, ob man sowas auslagert in ein tanss-modul. vllt könnten wir uns da auch mal was mit ki basteln, um die tanss-api mit python zu bedienen, so eine standard-library. das könnte für odoo auch gut werden. aber für jetzt: vielleicht in ein tanss/utilities modul oder so, dann kann man das wiederverwenden für zukünftige projekte
Author
Member

Ich habe mich dagegen entschieden, das in den tanss ordner zu verschrieben, weil es eine sehr generische funktion ist und nicht viel mit tanss zu tun hat

Ich habe mich dagegen entschieden, das in den tanss ordner zu verschrieben, weil es eine sehr generische funktion ist und nicht viel mit tanss zu tun hat
shuber marked this conversation as resolved
src/logger.py Outdated
@ -0,0 +4,4 @@
import sys
logging.basicConfig(
level=logging.INFO,
Owner

loglevel sollte setzbar über umgebungsvariable sein

loglevel sollte setzbar über umgebungsvariable sein
dgoetz marked this conversation as resolved
@ -0,0 +9,4 @@
handlers=[logging.StreamHandler(sys.stdout)],
)
logger = logging.getLogger("ninja-tanss")
Owner

den logger-namen könnten wir vllt umbennen zum app-namen

den logger-namen könnten wir vllt umbennen zum app-namen
dgoetz marked this conversation as resolved
@ -0,0 +28,4 @@
Priority.CRITICAL: logging.ERROR,
Priority.HIGH: logging.WARNING,
Priority.MEDIUM: logging.INFO,
Priority.LOW: logging.DEBUG,
Owner

bitte kein debug für logs für diese flughöhe der logik verwenden. debug soll ja eher helfen das programm zu verstehen, das soll ja hinweisen, dass was nicht hinhaut mit dem datenbestand

bitte kein debug für logs für diese flughöhe der logik verwenden. debug soll ja eher helfen das programm zu verstehen, das soll ja hinweisen, dass was nicht hinhaut mit dem datenbestand
dgoetz marked this conversation as resolved
@ -0,0 +22,4 @@
from ..logger import logger
from .oauth2 import oauth
# Hardcoded endpoints
Owner

die base_url sollten wir auslagern. man weiß ja nie

die base_url sollten wir auslagern. man weiß ja nie
dgoetz marked this conversation as resolved
@ -0,0 +137,4 @@
url = f"{NINJA_API_BASE_URL}{NINJA_CUSTOM_FIELDS_ENDPOINT.format(id=device_id)}"
for attempt in range(max_retries):
Owner

diese retry-logik würde ich auch in einer anderen methode sehen. dann kann man das recyclen

diese retry-logik würde ich auch in einer anderen methode sehen. dann kann man das recyclen
Author
Member

Die wird aktuell nur in der Datei benötigt. Ich würde das erst auslagern, wenn es noch einen Use-Case dafür gibt.

Die wird aktuell nur in der Datei benötigt. Ich würde das erst auslagern, wenn es noch einen Use-Case dafür gibt.
Owner

iO

iO
shuber marked this conversation as resolved
@ -0,0 +212,4 @@
device.tanssId = fields[0]
device.teamviewerId = fields[1]
saveCache("ninja_devices.json", [d.__dict__ for d in result])
Owner

wie besprochen

wie besprochen
dgoetz marked this conversation as resolved
@ -0,0 +32,4 @@
return match.group(1) if match else None
class OrganizationSchema(Schema):
Owner

auch hier könnten wir das vielleicht bisschen ausbauen und in eine ninja library packen. das wäre natürlich cool, weil wir das für odoo auch recyclen könnten.

auf jeden fall auslagern, wenn möglich. weißt du wie ich mein?

auch hier könnten wir das vielleicht bisschen ausbauen und in eine ninja library packen. das wäre natürlich cool, weil wir das für odoo auch recyclen könnten. auf jeden fall auslagern, wenn möglich. weißt du wie ich mein?
Author
Member

Ich weiß gerade nicht, wie ich das umsetzen soll. Das würde bedeuten, dass man zwei neue Repos für tanss und ninja anlegt und diese dann irgendwie über uv runterlädt und dann importiert?

Ich weiß gerade nicht, wie ich das umsetzen soll. Das würde bedeuten, dass man zwei neue Repos für tanss und ninja anlegt und diese dann irgendwie über uv runterlädt und dann importiert?
Owner

später einmal, und du könntest den grundstein dafür legen, genau
einfach alles ninja-ige möglichst generisch schreiben und in ein modul, das man später rausnehmen kann

später einmal, und du könntest den grundstein dafür legen, genau einfach alles ninja-ige möglichst generisch schreiben und in ein modul, das man später rausnehmen kann
dgoetz marked this conversation as resolved
@ -0,0 +13,4 @@
#
# Hardcoded endpoint
NINJA_API_BASE_URL = "https://datajob.rmmservice.eu"
Owner

das könnte man zu einer variable machen

das könnte man zu einer variable machen
dgoetz marked this conversation as resolved
@ -0,0 +47,4 @@
tanss_display_id = ninja_org.get_tanss_display_id()
if tanss_display_id is None:
if ninja_org.name == "Meißner EDV - Kunden":
Owner

könnten wir den namen in eine variable speichern und nicht hardcoden?

könnten wir den namen in eine variable speichern und nicht hardcoden?
Author
Member

Ich weiß nicht, ob das den Aufwand und die Logik wert ist.
Das hier soltle ein Einzelfall sein und auf absehbare zeit auch bleiben

Ich weiß nicht, ob das den Aufwand und die Logik wert ist. Das hier soltle ein Einzelfall sein und auf absehbare zeit auch bleiben
Owner

ist tatsächlich aber trotzdem eleganter, dann kann man nämlich auch die variable benennen und einen schönen kommentar hinzufügen. so denk ich mir: was geht denn hier ab

ist tatsächlich aber trotzdem eleganter, dann kann man nämlich auch die variable benennen und einen schönen kommentar hinzufügen. so denk ich mir: was geht denn hier ab
dgoetz marked this conversation as resolved
@ -0,0 +50,4 @@
if ninja_org.name == "Meißner EDV - Kunden":
# => that's ok
continue
elif ninja_org.name == "Neue Geräte":
Owner

s. oben

s. oben
dgoetz marked this conversation as resolved
@ -0,0 +17,4 @@
logger = logging.getLogger(__name__)
def phase2(
Owner

ich hab ja schon gesagt, dass ich mit den namen nicht so zufrieden bin. vielleicht könntest du da was anderes finden als phaseX, das ein bisschen ausdrucksstärker ist

ich hab ja schon gesagt, dass ich mit den namen nicht so zufrieden bin. vielleicht könntest du da was anderes finden als phaseX, das ein bisschen ausdrucksstärker ist
dgoetz marked this conversation as resolved
@ -0,0 +142,4 @@
days_since_contact = (datetime.now().date() - last_contact_date).days
if days_since_contact > 90: # 3 months
logger.debug(
Owner

hier debug vllt wie gesagt abändern

und die logs kommen ja nie durch, loglevel ist ja auf "info" gesetzt

hier debug vllt wie gesagt abändern und die logs kommen ja nie durch, loglevel ist ja auf "info" gesetzt
Author
Member

Mittlerweile kann man das log level in der .env einstellen. Soll ich es dennoch ändern?

Mittlerweile kann man das log level in der .env einstellen. Soll ich es dennoch ändern?
Owner

ja bitte

ja bitte
dgoetz marked this conversation as resolved
@ -0,0 +166,4 @@
)
continue
else:
if tanss_device.is_peripherie:
Owner

englischer begrifff bitte (periphery)

englischer begrifff bitte (periphery)
dgoetz marked this conversation as resolved
@ -0,0 +42,4 @@
pass
elif tanss_device.teamviewerId != "" and ninja_device.teamviewerId is not None:
if tanss_device.teamviewerId != ninja_device.teamviewerId:
logger.debug(f"TeamviewerId mismatch for {tanss_device=} and {ninja_device=}")
Owner

natürlich hier dann auch überall logger.debug ersetzen

natürlich hier dann auch überall logger.debug ersetzen
dgoetz marked this conversation as resolved
@ -0,0 +14,4 @@
from .devices import getTanssDevices
USERNAME = "user"
PASSWORD = "HIwURXsB02OECXkk37W5"
Owner

🫣

🫣
Author
Member

Upsi

Upsi
dgoetz marked this conversation as resolved
@ -0,0 +55,4 @@
def getTanssCompany(company_id: str) -> TanssCompany:
"""Fetch a single TANSS company by company_id."""
response = get(
f"https://nodered.dev.datajob.de/tanss-wrapper/v1/companies/{company_id}",
Owner

hier kann man die url auch konfigurierbar machen

hier kann man die url auch konfigurierbar machen
dgoetz marked this conversation as resolved
@ -0,0 +159,4 @@
next_batch: list[int] = list(tanss_company_ids)
# Fetch companies and their linked companies in batches using threads
with tqdm(unit="company") as pbar:
Owner

da wären aussagekräftigere namen besser.
gut ist, wenn der code innerhalb 10 sekunden klar wird und man den kontext aus den namen ableiten kann - das hat hier leider nicht geklappt

EDIT: ich sehe, dass tqdm der name einer library. dann vllt efinach progress_bar statt pbar und einen kommentar

da wären aussagekräftigere namen besser. gut ist, wenn der code innerhalb 10 sekunden klar wird und man den kontext aus den namen ableiten kann - das hat hier leider nicht geklappt EDIT: ich sehe, dass tqdm der name einer library. dann vllt efinach progress_bar statt pbar und einen kommentar
dgoetz marked this conversation as resolved
@ -0,0 +98,4 @@
A list of TanssContract objects containing id, from_date, to_date, and contract_name.
"""
response = get(
f"https://ticket.datajob.de/ajax/srv/gv_v4.php?t=showWvTasksInfo&type=1&id={device_id}",
Owner

URL umgebungsvariable

URL umgebungsvariable
dgoetz marked this conversation as resolved
@ -0,0 +157,4 @@
devices = [d for d in getTanssDevices() if not d.is_peripherie]
all_contracts: list[TanssContract] = []
printFetching(f"Fetching TANSS contracts: {len(devices)} items with {MAX_WORKERS} workers")
Owner

das muss man vereinheitlichen, auch über logging.info

das muss man vereinheitlichen, auch über logging.info
Author
Member

Was meinst du genau? Mittlerweile wurde printFetching durch logger.info ersetzt

Was meinst du genau? Mittlerweile wurde printFetching durch logger.info ersetzt
Owner

genau das meint ich!

genau das meint ich!
dgoetz marked this conversation as resolved
@ -0,0 +204,4 @@
Returns:
The matching Contract enum value, or UNKNOWN if no match.
"""
if (
Owner

das lieber als variablen speichern bitte

das lieber als variablen speichern bitte
dgoetz marked this conversation as resolved
@ -0,0 +10,4 @@
from ..helper import printFetching
# Hardcoded base URL
TANSS_API_BASE_URL = "https://nodered.dev.datajob.de"
Owner

env-var hier wieder

EDIT: ah und der name ist irreführend: das ist ja nicht die TANSS_API, das ist der node-red wrapper

env-var hier wieder EDIT: ah und der name ist irreführend: das ist ja nicht die TANSS_API, das ist der node-red wrapper
dgoetz marked this conversation as resolved
@ -0,0 +1,651 @@
<!DOCTYPE html>
Owner

kannst du der ki vllt sagen, sie soll separate css und js-dateien nehmen? so ist das ja grausam

kannst du der ki vllt sagen, sie soll separate css und js-dateien nehmen? so ist das ja grausam
Author
Member

Die index.html ist eigentlich auch nicht zum Verstehen da
Aber ich geb's weiter :D

Die index.html ist eigentlich auch nicht zum Verstehen da Aber ich geb's weiter :D
dgoetz marked this conversation as resolved
Owner

@dgoetz ist fertig
@opencode siehst du noch etwas?

@dgoetz ist fertig @opencode siehst du noch etwas?
shuber merged commit 838a7ce34e into main 2026-08-21 06:01:41 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
GENERAL/syncninja!1
No description provided.