search

PnX-SI / UsersHub-authentification-module

Module Flask d'authentification de UsersHub
GNU General Public License v3.0
5 stars 13 forks source link

Ne pas dupliquer le script de création de la BDD #2

Closed camillemonchicourt closed 5 years ago

camillemonchicourt commented 7 years ago

Faire appel au SQL qui est dans le dépôt de UsersHub : https://github.com/PnEcrins/UsersHub/blob/master/data/usershub.sql

Car sinon on doit maintenir le SQL dans les 2 projets.

ksamuel commented 7 years ago

@camillemonchicourt @amandine-sahl @samuelpriou @gildeluermoz

Nous avons tenté d'installer le projet sur un serveur du PNM. Cependant, ils utilisent postgres 9.4, qui ne supporte pas l'instruction "IF NOT EXISTS" dans "CREATE SEQUENCE". La migration vers du 9.5 representerait une somme non négligeable de travail.

Il existe une manière de contourner le problème, en remplaçant les instructions de type :

CREATE SEQUENCE IF NOT EXISTS schema.nom
  INCREMENT 1
  MINVALUE 1
  MAXVALUE 9223372036854775807
  START 1
  CACHE 1;

Par:

DO
$$
BEGIN
  CREATE SEQUENCE schema.nom
    INCREMENT 1
    MINVALUE 1
    MAXVALUE 9223372036854775807
    START 1
    CACHE 1;
EXCEPTION WHEN duplicate_table THEN
        -- do nothing, it's already there
END
$$;

Puis-je créer une PR dans ce sens ? Attention, cela rendrait le SQL utilisé pour initialiser UsersHub-authentification-module et UsersHub différents. Et donc une raison supplémentaire de mettre en place une seule source pour le récupérer plutot que la duplication actuelle.

gildeluermoz commented 7 years ago

Lorsque l'on fait évoluer UsersHub, et si cela nécessite une évolution de la base, nous proposons un script update1.xto1.y.sql. Il est donc important que les différentes bases installées ne diffèrent pas pour que le script fonctionne. La modification proposée ne génère pas de différence sur la base. Mais maintenir 2 scripts peut aboutir à ce résultat. Effectivement cela génère une différence de script. Je propose qu'il n'y ait effectivement qu'un seul script. Actuellement, la création de la base GeoNature appel le script de UsersHub : https://github.com/PnEcrins/GeoNature/blob/master/install_db.sh#L49-L54 idem pour l'installation de la taxonomie avec TaxHub : https://github.com/PnEcrins/GeoNature/blob/master/install_db.sh#L57-L66 Si votre contexte s'y prête, il serait préférable de s'inspirer de ce concept permettant de ne pas dupliquer les scripts de création du schéma utilisateur.

Je ne comprends pas la nécessité du IF NOT EXIST dans un script de création. Est-ce que le supprimer n'est pas la solution la plus simple. Cette instruction n'est pas présente sur le script initial de création de la base UsersHub. Est-ce que CREATE OR REPLACE fonctionnerait ?

ksamuel commented 7 years ago

Effectivement cela génère une différence de script. Je propose qu'il n'y ait effectivement qu'un seul script. Actuellement, la création de la base GeoNature appel le script de usersHub : https://github.com/PnEcrins/GeoNature/blob/master/install_db.sh#L49-L54 idem pour l'installation de la taxonomie avec TaxHub : https://github.com/PnEcrins/GeoNature/blob/master/install_db.sh#L57-L66 Si votre contexte s'y prête, il serait préférable de s'inspirer de ce concept permettant de ne pas dupliquer les scripts de création du schéma utilisateur.

On ne peut pas dépendre de bash pour l'installation de l'application, car certains developpeurs le feront sur Windows.

Je ne comprends pas la nécessité du IF NOT EXIST dans un script de création. Est-ce que le supprimer n'est pas la solution la plus simple.

Le but est de permettre au script d'être idempotent et de pouvoir être relancé plusieurs fois d'affiler sans causer d'erreur. C'est surtout utile en développement.

Est-ce que CREATE OR REPLACE fonctionnerait ?

Cela supprimerait la référence existante ce qui n'est pas le but. Le but est de ne rien faire.

Mais je suis tout à fait d'accord pour avoir une seule source pour le schéma. Pour cela l'idéal serait d'isoler les parties du schéma qui ne concernent que l'authentification et le mettre à disposition dans un endroit qui ne requière pas de télécharger tout UsersHub pour l'obtenir et chercher si on a la bonne version du schéma.

Dans ces conditions je peux modifier UsersHub-authentification-module pour télécharger le schéma à la première initialisation.

Je pense que la solution idéale serait pour UsersHub d'exposer une URL qui retourne le schéma actuel. L'avantage de ce système est que:

gildeluermoz commented 7 years ago

Ok, je comprends le contexte. De notre coté, on ne souhaite pas gérer des compatibilités Windows, et nous n'avons pas le temps nécessaire pour simplifier à ce point le travail des développeurs. Quand on test une install, on déroule un install_app, install_db, éventuellement un install_env. Ceci écrase la base de test. Si on est en prod on propose des scripts de mise à jour sql pour la base et une procédure de mise à jour pour le code de l'application et sa config. Pour le moment, nous n'avons ni les compétences ni les moyens d'aller plus loin.

Je pense que la solution idéale serait pour UsersHub d'exposer une URL qui retourne le schéma actuel. Le script proposé n'installe pas toute la base mais uniquement le schéma utilisateur et cette url retourne le script SQL brut : https://raw.githubusercontent.com/PnEcrins/UsersHub/master/data/usershub.sql

ksamuel commented 7 years ago

Ok, donc on pourrait modifier ce script pour rajouter les "IF NOT EXISTS" (ou emulations) et l'exposer sur une URL ? Il n'y a pas besoin de faire plus.

gildeluermoz commented 7 years ago

Donc si les IF NOT EXISTS ne sont pas compatible sur les versions avant la 9.5, on ne peut pas le mettre. Il faut donc mettre l'émulation. Je ne connais pas cette formulation. Ca ressemble à du plggsql. Si on met ça, dans le fichier SQL, psql saura l'interpréter ?

DO
$$
BEGIN
  CREATE SEQUENCE schema.nom
    INCREMENT 1
    MINVALUE 1
    MAXVALUE 9223372036854775807
    START 1
    CACHE 1;
EXCEPTION WHEN duplicate_table THEN
        -- do nothing, it's already there
END
$$;
gildeluermoz commented 7 years ago

Ok, testé dans pgadmin, ça fonctionne. Je regarde le script de UserHub et je vous tiens au courant.

ksamuel commented 7 years ago

Super. Le "IF NOT EXISTS" marche pour tout le reste en 9.4: tables, schemas, etc. Juste pas pour les sequences.

Pour l'URL, si le repository de UsersHubs est proprement taggué à chaque changement de version, je peux juste référencer l'URL de la version en cours. Ex:

https://github.com/PnEcrins/UsersHub/blob/1.2.0/data/usershub.sql

Ce qui permet de gérer les différentes versions de schéma de usershub de mon côté et vous évite de faire une modification du code PHP de usershub pour exposer le fichier SQL en live. Par contre il ne faut pas que le repo change de nom, d'adresse et que les tags restent à jour.

gildeluermoz commented 7 years ago

Voici une proposition https://github.com/PnEcrins/UsersHub/commit/9b40f7459a1fa6b4401552dc4d2a79fde8a133d4 Dites moi si le fichier sql convient : https://raw.githubusercontent.com/PnEcrins/UsersHub/master/data/usershub.sql

Je l'ai testé sur une base existante et ça fonctionne bien

ksamuel commented 7 years ago

Ok ça à une très bonne tête. Je vais tenter l'intégration cette après midi (download automatique, execution, etc). Je fais un retour dessus dès que j'ai tout testé.

camillemonchicourt commented 7 years ago

Nous venons d'intégrer cela dans la nouvelle version de UsersHub : https://github.com/PnEcrins/UsersHub/releases/tag/1.2.1

camillemonchicourt commented 6 years ago

Fichiers SQL locaux à supprimer pour utiliser ceux dans le Github de UsersHub :

camillemonchicourt commented 5 years ago

Pour le moment, on a au moins indiqué dans la DOC de ne pas utiliser le SQL local : https://github.com/PnX-SI/UsersHub-authentification-module/commit/0745478ece58fd7507fa708f2e5f32718307d16a

Par contre désormais, si on exécute uniquement le SQL de création du schéma utilisateurs on rencontre une erreur car il manque l'extension UUID qui est créé par le script install_db.sh : https://github.com/PnEcrins/UsersHub/blob/master/install_db.sh

CREATE EXTENSION IF NOT EXISTS "uuid-ossp

gildeluermoz commented 5 years ago

On peut le mettre dans le sh ?

camillemonchicourt commented 5 years ago

Il y est dans le sh, mais du coup c'est pas utilisé si on exécute que le SQL.

gildeluermoz commented 5 years ago

Du coup tu voudrais qu'on l'ajoute dans le sql ?

camillemonchicourt commented 5 years ago

Non ça me semble pas possible car il faut etre super-utilisateur pour ça.

Non c'est juste pour garder en tête qu'il faudra revoir la méthode ou la doc pour ceux qui installent le module dans UsersHub.

camillemonchicourt commented 5 years ago

SQL du schéma retiré du sous-module : https://github.com/PnX-SI/UsersHub-authentification-module/commit/66bb152f1e5930dfac15d7764de7644ab834a342 Et doc complétée.