Skip to content

Statische Dependencies Laden #165

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Statische Dependencies Laden #165

wants to merge 5 commits into from

Conversation

MHajoha
Copy link
Member

@MHajoha MHajoha commented Jul 15, 2025

Ich trenne den Lebenszyklus von Paketen jetzt in 2 Schritte. Erst wird ein Paket geöffnet aka prepared, im Falle von ZIP-Paketen geht damit das Entpacken einher. Dann wird das Manifest gelesen und anhand dessen werden alle Dependencies geöffnet (rekursiv). Wenn der gesamte Baum geöffnet wurde, werden alle Pakete linearisiert und initialisiert.

Die Trennung zwischen dem Öffnen und dem Initialisieren der Pakete halte ich an sich für sauber, aber erlaubt auch in Zukunft, dazwischen noch weitere Nachrichten hin-und her zu passen. Beispielsweise könnten dynamische Dependencies mit einer "Ich brauche noch folgende Pakete"-Nachricht vom Worker zum Server geschehen.

Außerdem fallen etwaige Dependency-Konflikte dann auf, bevor irgendein Paket initialisiert wird.

Closes: #158

super().__init__(f"'{nssn}'. Dependency stack: {stack}", stack)


class TooDeeplyNestedDependencyError(Exception):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class TooDeeplyNestedDependencyError(Exception):
class TooDeeplyNestedDependencyError(DependencyError):


new_stack = (*stack, nssn)
for dep_location in package.resolve_static_dependencies():
dep_nssn, dep_package = self._open_packages_recursively(msg, dep_location, new_stack)
Copy link
Contributor

@janbritz janbritz Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Würde das nicht kaputt gehen, wenn z.B. Paket A zwei Dependencies hat (X und Y v2.0.0) und X hat als Dependency Y v1.0.0, wobei Y v1.0.0 und Y 2.0.0 inkompatibel sind?

Ah, dieser Kommentar müsste vermutlich nur angepasst werden: Es werden aktuell ja keine doppelten Pakete erlaubt, egal welche Version sie haben und wo sie liegen.

Comment on lines +184 to +189
class PackageNotInitializedError(Exception):
"""The packages state was not INITIALIZED."""


class PackageNotLoadedError(Exception):
"""The packages state was not LOADED or higher."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class PackageNotInitializedError(Exception):
"""The packages state was not INITIALIZED."""
class PackageNotLoadedError(Exception):
"""The packages state was not LOADED or higher."""
class PackageNotInitializedError(Exception):
"""The package's state was not INITIALIZED."""
class PackageNotLoadedError(Exception):
"""The package's state was not LOADED or higher."""

Comment on lines +171 to +172
if nssn in stack and self._packages[nssn].manifest.version == package.manifest.version:
raise CircularDependencyError(nssn, stack)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich denke, diese Erkennung funktioniert nur innerhalb eines einzelnen Paket-Lade-Zyklus, oder? Wenn mehrere Pakete nacheinander geladen werden mittels LoadQPyPackage(...) nicht mehr. Konkretes Beispiel:

  1. LoadQPyPackage(A)
A > B

✔️ (da kein Zyklus im aktuellen stack)

  1. LoadQPyPackage(B)
B > C

✔️ (da kein Zyklus im aktuellen stack)

  1. LoadQPyPackage(C)
C > A

✔️ (da kein Zyklus im aktuellen stack)

TopologicalSorter.static_order() wirft CycleError, da über alle aktuell geladenen self._packages sortiert wird und insgesamt aber A > B > C > A (zyklisch).

Also, müsste static_order() doch noch ein try/catch bekommen, oder?

    try:
        linearized = _linearize_packages(self._packages)
    except CyclicError:
        raise CircularDependencyError(...)

Comment on lines +125 to +127
class DistStaticQPyDependency(BaseModel):
name: str
hash: str
Copy link
Contributor

@tumidi tumidi Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Evtl. das noch inline dokumentieren.

Ich hatte naiv erst mal angenommen, es handelt sich bei name um den Package-Namen, bzw. -Specifier. Aber es handelt sich quasi um eine "filename-friendly" Variante, oder?

Vielleicht kann die Generierung des Namens auch direkt in Manifest passieren?

    @property
    def dirname(self) -> str:
        return f"{self.namespace}-{self.short_name}-{self.version}"

Oder so ähnlich. Dann muss das SDK hier auch nicht wissen, wie dieser Name zu generieren ist.

Btw, das ließe sich dann auch im SDK Webserver gleich mitverwenden, der generiert auch so einen "dateinamenfreundlichen" Namen hier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Statische Paketabhängigkeiten laden Statische Paketabhängigkeiten paketieren
3 participants