-
Notifications
You must be signed in to change notification settings - Fork 102
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
Alembic - SQL et lisibilité de la BDD et de ses évolutions #1574
Comments
Oui tout cela nous a valu pas mal de questions, discussions et débats. Donc on avait différents scripts SQL organisés par schémas pour créer la BDD de GeoNature : https://github.com/PnX-SI/GeoNature/tree/master/data A chaque évolution de la BDD on faisait évoluer manuellement ces scripts de création de la BDD, ainsi que des scripts SQL de migration pour que ceux qui ont déjà la BDD puissent la mettre à jour. C'était une approche SQL très lisible pour les administrateurs de BDD mais aussi assez lourde et fragile, potentiellement source d'erreurs, de différences entre la création de la BDD et sa mise à jour, de mises à jour partielles de la BDD, de difficultés pour les tests automatisés, des migrations dans des PR qui ne correspondaient plus aux bonnes versions au moment où on les mergeait.... Cela rendait aussi le déploiement et la mise à jour de GeoNature plus complexe pour les administrateurs qui devaient exécuter manuellement les différents scripts SQL, etc... C'est pourquoi on avait d'abord initié un développement maison pour automatiser l’exécution de ces scripts, avant de privilégier une solution existante, plus robuste, maintenue et dédiée à cela qu'est Alembic : #880 Avec Alembic on est passé à une méthode de gestion de la BDD et de ses évolutions, beaucoup plus orientée python. Cependant les débats et discussions ont porté notamment sur différents aspects que tu mets en avant :
Les pistes pour répondre à cela tout en bénéficiant des évolutions et de la puissance apportée par le passage à Alembic :
DOC Alembic : http://docs.geonature.fr/admin-manual.html#administration-avec-alembic ? |
Si une table est supprimé, un grep le trouvera aussi bien dans un fichier alembic que dans un fichier migrate_xxx_to_yyy.sql, sans avoir besoin de lancer de commande. |
Je partage la plupart des avis exprimés ici : je comprend tout à fait les avantages qu'offre Alembic pour les déploiements et mises à jours, mais les évolutions de la BDD deviennent assez opaques pour une application à l'origine "DB first" |
La nouvelle commande Ci-dessous un exemple qui est moins lisible que l'original car il ne contient pas la coloration syntaxique:
Avec le résultat ci-dessus, il devient plus facile de demander le SQL correspondant à la migration de la révision geonature db upgrade 7471f51011c8:6f7d5549d49e -x local-srid="4326" --sql > migration.sql Le fichier SQL BEGIN;
-- Running upgrade 7471f51011c8 -> 9a9f4971edcd
CREATE OR REPLACE FUNCTION ref_geo.fct_trg_calculate_alt_minmax()
RETURNS trigger
LANGUAGE plpgsql
AS $function$
DECLARE
the4326geomcol text := quote_ident(TG_ARGV[0]);
thelocalsrid int;
BEGIN
-- si c'est un insert et que l'altitude min ou max est null -> on calcule
IF (TG_OP = 'INSERT' and (new.altitude_min IS NULL or new.altitude_max IS NULL)) THEN
--récupérer le srid local
SELECT INTO thelocalsrid parameter_value::int FROM gn_commons.t_parameters WHERE parameter_name = 'local_srid';
--Calcul de l'altitude
SELECT (ref_geo.fct_get_altitude_intersection(st_transform(hstore(NEW)-> the4326geomcol,thelocalsrid))).* INTO NEW.altitude_min, NEW.altitude_max;
-- si c'est un update et que la geom a changé
-- on vérifie que les altitude ne sont pas null
-- OU si les altitudes ont changé, si oui = elles ont déjà été calculés - on ne relance pas le calcul
ELSIF (
TG_OP = 'UPDATE'
AND NOT public.ST_EQUALS(hstore(OLD)-> the4326geomcol, hstore(NEW)-> the4326geomcol)
and (new.altitude_min = old.altitude_max or new.altitude_max = old.altitude_max)
and not(new.altitude_min is null or new.altitude_max is null)
) then
--IF (new.altitude_min is null or new.altitude_max is null) OR (NOT OLD.altitude_min = NEW.altitude_min or NOT OLD.altitude_max = OLD.altitude_max) THEN
--récupérer le srid local
SELECT INTO thelocalsrid parameter_value::int FROM gn_commons.t_parameters WHERE parameter_name = 'local_srid';
--Calcul de l'altitude
SELECT (ref_geo.fct_get_altitude_intersection(st_transform(hstore(NEW)-> the4326geomcol,thelocalsrid))).* INTO NEW.altitude_min, NEW.altitude_max;
--end IF;
--else
END IF;
RETURN NEW;
END;
$function$
;;
UPDATE alembic_version SET version_num='9a9f4971edcd' WHERE alembic_version.version_num = '7471f51011c8';
-- Running upgrade 9a9f4971edcd -> 6f7d5549d49e
DROP VIEW gn_commons.v_synthese_validation_forwebapp;;
UPDATE alembic_version SET version_num='6f7d5549d49e' WHERE alembic_version.version_num = '9a9f4971edcd';
COMMIT; J'ai aussi réussi à récupérer tout le SQL d'une branche comme ceci: geonature db upgrade nomenclatures_inpn_data --sql > migration.sql |
Intégration de la commande |
Salut tout le monde, Petite réflexion concernant l'utilisation d'alembic notamment pour la gestion de vues , fonction : A l'heure actuelle , il y a pas mal de migrations alembic qui sont réalisées en écrivant du texte sql directement dans les fonctions Lines 19 to 112 in 22b577a
La documentation d'alembic propose dans sa documentation une gestion des vues , procédures sql. L'autre point concerne l'écriture en dur pour la suppression / creation de vues sql dans chaque fichier de migration alembic ou bien d'appeler un fichier sql créé à part et qui serait appelé dans les fichiers de migrations alembic. Concernant les opérations custom proposées par alembic ça donnerait un truc comme ça : Un fichier où l'on créé les opérations alembic custom fichier alembic_utilsfrom alembic.operations import Operations, MigrateOperation
class ReplaceableObject:
def __init__(self, name, sqltext):
self.name = name
self.sqltext = sqltext
class ReversibleOp(MigrateOperation):
def __init__(self, target):
self.target = target
@classmethod
def invoke_for_target(cls, operations, target):
op = cls(target)
return operations.invoke(op)
def reverse(self):
raise NotImplementedError()
@classmethod
def _get_object_from_version(cls, operations, ident):
version, objname = ident.split(".")
module = operations.get_context().script.get_revision(version).module
obj = getattr(module, objname)
return obj
@classmethod
def replace(cls, operations, target, replaces=None, replace_with=None):
if replaces:
old_obj = cls._get_object_from_version(operations, replaces)
drop_old = cls(old_obj).reverse()
create_new = cls(target)
elif replace_with:
old_obj = cls._get_object_from_version(operations, replace_with)
drop_old = cls(target).reverse()
create_new = cls(old_obj)
else:
raise TypeError("replaces or replace_with is required")
operations.invoke(drop_old)
operations.invoke(create_new)
@Operations.register_operation("create_view", "invoke_for_target")
@Operations.register_operation("replace_view", "replace")
class CreateViewOp(ReversibleOp):
def reverse(self):
return DropViewOp(self.target)
@Operations.register_operation("drop_view", "invoke_for_target")
class DropViewOp(ReversibleOp):
def reverse(self):
return CreateViewOp(self.target)
@Operations.implementation_for(CreateViewOp)
def create_view(operations, operation):
operations.execute("CREATE VIEW %s AS %s" % (operation.target.name, operation.target.sqltext))
@Operations.implementation_for(DropViewOp)
def drop_view(operations, operation):
operations.execute("DROP VIEW %s" % operation.target.name) Et ensuite on utilise de cette manière dans les fichiers de migrations alembic 1ere revision alembic qui modifie la vue initiale"""add id_module to v_synthese_for_web_app
Revision ID: 446e902a14e7
Revises: f1dd984bff97
Create Date: 2023-09-25 10:09:39.126531
"""
import importlib
from alembic import op
from sqlalchemy.sql import text
from geonature.utils import alembic_utils
# revision identifiers, used by Alembic.
revision = "446e902a14e7"
down_revision = "f1dd984bff97"
branch_labels = None
depends_on = None
view_name = "gn_synthese.v_synthese_for_web_app"
path_synthese = "geonature.migrations.data.core.gn_synthese"
init_filename = "initial_v_synthese_for_web_app_v1.0.0.sql"
sql_text = text(importlib.resources.read_text(path_synthese, init_filename))
v_synthese_for_web_app_init = alembic_utils.ReplaceableObject(view_name, sql_text)
filename = "v_synthese_for_web_app_add_id_module_v1.0.1.sql"
sql_text = text(importlib.resources.read_text(path_synthese, filename))
v_synthese_for_web_app = alembic_utils.ReplaceableObject(view_name, sql_text)
def upgrade():
op.drop_view(v_synthese_for_web_app_init)
op.create_view(v_synthese_for_web_app)
def downgrade():
op.drop_view(v_synthese_for_web_app)
op.create_view(v_synthese_for_web_app_init) 2nde revision alembic qui modifie une nouvelle fois la vue de la précédente revision"""add_column_group_inpn_to_v_synthese_for_web_app
Revision ID: d99a7c22cc3c
Revises: 446e902a14e7
Create Date: 2023-11-17 14:53:42.138762
"""
import importlib
from alembic import op
from sqlalchemy.sql import text
from geonature.utils import alembic_utils
# revision identifiers, used by Alembic.
revision = "d99a7c22cc3c"
down_revision = "446e902a14e7"
branch_labels = None
depends_on = ("c4415009f164",) # Taxref v15 db structure
view_name = "gn_synthese.v_synthese_for_web_app"
path_synthese = "geonature.migrations.data.core.gn_synthese"
filename = "v_synthese_for_web_app_add_group_inpn_v1.0.2.sql"
sql_text = text(importlib.resources.read_text(path_synthese, filename))
v_synthese_for_web_app = alembic_utils.ReplaceableObject(view_name, sql_text)
def upgrade():
# on fait référence ici à la vue de la revision précédente en spécifiant l'identifiant de la revision
op.replace_view(v_synthese_for_web_app, replaces="446e902a14e7.v_synthese_for_web_app")
def downgrade():
op.replace_view(v_synthese_for_web_app, replace_with="446e902a14e7.v_synthese_for_web_app") A discuter , réflechir concernant une bonne pratique à mettre en place pour la gestion d'alembic |
Comme j'indiquai dans la PR où on a parlé du sujet (#2637 (comment)), je ne ferai pas un fichier SQL par changement, au contraire. |
Plutôt intéressé par le fait de centraliser les triggers, les fonctions et les vues (TFV pour la suite) dans des fichiers uniques. Toutefois, il faudra accompagnez cela d'une commande |
Retours et questions sur le passage à Alembic et la lisibilité des évolutions de la BDD :
@jbrieuclp :
@camillemonchicourt :
@jbrieuclp :
@TheoLechemia :
The text was updated successfully, but these errors were encountered: