-
Notifications
You must be signed in to change notification settings - Fork 3
feat: migrations #251
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
base: dev
Are you sure you want to change the base?
feat: migrations #251
Conversation
96c78aa to
0190efe
Compare
0190efe to
0d7d3b0
Compare
17f414d to
8bb58cd
Compare
8bb58cd to
0bdaf1c
Compare
MartinGauk
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.
Welchen Vorteil hat es, die einzelnen Migrations in Klassen zu stecken, die nur aus einer Methode bestehen?
| # TODO: Log (?) the full exception for further investigations. | ||
| raise PackageBuildError(msg) from e | ||
|
|
||
| self._manifest.state_version = len(migrations.package) |
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.
Hier wäre es besser statt nach der Anzahl nach dem höchsten/max Integer zu sehen. Falls man doch alte Migrations mal löscht, sollte das nichts durcheinander bringen.
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.
Was genau meinst du mit "höchsten/max Integer"? Ich hatte das damals im Meeting angesprochen, dass wir es erlauben sollten, Migrations zu löschen (Digital Waste / Übersichtlichkeit) - wir sind dann aber zum Schluss gekommen, dass wir das nicht wollen. Mit der jetztigen Implementierung könnten auch alle alten Migrations in einer Datei "versteckt" werden.
| except Exception as e: | ||
| raise SpecificMigrationFailedError( | ||
| e, migration_state.state_version, migration_state.state_version + 1, step | ||
| ) from e |
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.
Hier muss es dem Paket möglich sein, eigene Fehlermeldungen zurückzugeben.
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.
Das ist möglich wenn die Exception e vom Typ MigrationNotPossible ist (siehe).
Mir fällt gerade auf das hier SpecificMigrationError besser als SpecificMigrationFailedError passt.
919262b to
6740b8b
Compare
Basiert auf questionpy-org/questionpy-server#195
Migrations können folgendermaßen erstellt werden:
Diese müssen in einem Python-Package mit dem Namen
migrationsdefiniert werden.Migrations werden nach dem Namen der Python-Datei, in welcher sie definiert wurden, sortiert. Es können auch mehrereMigrations in einer Datei leben, da zusätzlich auch nach dem Klassen-Name (genauer__qualname__) sortiert wird.Beim Paketieren wird die
state_versionbasierend auf der Anzahl der Migrations berechnet.