Mod/prisma #733

Riron · 2020-11-30T17:50:52Z

Migration de Prisma à Prisma2.
It's here ! Même si le rebase me donne déjà des sueurs froides 😭

Code

Tous les appels ont été migré vers prisma2. Dans les faits, peu de changements.
Quelques points à noter tout de même:

le typing est légèrement plus strict
les champs Date doivent maintenant être passés comme des dates et non des string
sur les champs typés scalaires URL, on a vraiment des types URL et non des string. Ce n'est pas lié à cette update, donc je suis étonné qu'il ne se soit pas plaint avant. mais j'imagine que prisma appliquait un .toString() automatiquement avant de le sauvegarder donc on ne voyait pas le problème

DB

Plusieurs tables ont été modifiées pour avoir un modèle plus simple / logique. Prisma1 au début créait des tables N-N quelque soit la relation. Certains champs ont donc été modifiés pour simplifier le modèle et passer à du 1-N / 1-1 quand cela avait du sens.
Des champs référencant des foreign key ont également été suffixés par id pour clarifier.
Ces modifs sont faites dans le fichier sql prisma/migration.relations.sql

Le système de seed a été conservé. Au lieu de les runner via prisma, on peut les runner via ts-node directement normalement. (voir fichier seed.ts)

Test d'inté

Le point positif, c'est qu'ils tournent en environ 9min au lieu 30. Les tests sont indentiques, la manière de les runner a un poil changé.
Prisma n'a pas encore de système de gestion des migrations (enfin c'est en bêta), donc je ne me suis pas appuyé dessus. A la place, je stocke un dump de la DB qui est restauré. Pour clean entre chaque test, je déclare une function chargée de truncate toutes les tables. C'est je pense beaucoup plus rapide que le reset prisma1, et à mon sens ca explique en partie les différences du temps de run avec prisma1.
Pour générer un nouveau dump de DB utilisé dans les tests d'inté, j'ai rajouté un flag -g au run.sh.

Migrations

C'est le point manquant. Le système de migrations est encore en bêta donc je ne m'en suis pas servi. De toute manière pour faire l'upgrade de 1 vers 2, il faut forcément le faire directement en SQL.
2 solutions pour les migrations:

soit on commence à utiliser le système de prisma2, toujours en bêta
soit on plug un système de migration purement sql (style ca)

=> Pour l'instant je suis parti sur une lib de migration, j'avais des problèmes avec le système prisma qui voulait effacer toutes mes données. Pour upgrade vous pouvez run npm run update:dev dans le container.

back/src/commands/onboarding.helpers.ts

providenz · 2020-12-09T14:34:19Z

il reste le client prisma 1 généré dans generated qui bloque le build

Riron · 2021-01-11T17:01:55Z

En effet, j'ai ajouté une note dans le fichier CONTRIBUTING.md

benoitguigal

🚀 🚀 super nice
Il me semble qu'on a encore un prisma deploy qui traine dans le CI

back/integration-tests/db-deploy/deploy-db.sh

benoitguigal · 2021-01-12T14:01:02Z

back/integration-tests/helper.ts

@@ -31,7 +30,7 @@ export async function resetDatabase() {
  // We need a longer than 5sec timeout...
  jest.setTimeout(10000);

-  await promisify(exec)("prisma reset --force");
+  await prisma.$executeRaw("SELECT truncate_tables();");


super idée le truncate pour accélérer les tests

benoitguigal · 2021-01-12T14:08:02Z

back/package.json

    "dev": "concurrently npm:watch-node npm:watch-graphql",
-    "watch-node": "npx nodemon -L --watch src --exec \"ts-node\" src/index.ts",
+    "watch-node": "npx nodemon -L --watch src --exec \"ts-node\" -r tsconfig-paths/register src/index.ts",


je ne suis pas certain de comprendre à quoi sert -r tsconfig-paths/register ici

C'est pour que les imports TS non relatifs fonctionnent correctement. Ex import {smthing} from "src/prisma"

Spécifier le baseUrl dans le tsconfig ne suffit pas ? C'est dommage :/ D'ailleurs au niveau du baseUrl est-ce qu'on ne devrait pas avoir ./src plutôt que . ? Ça éviterait la redondance de src dans les imports.

Non il faut ca pour faire marcher le baseUrl avec ts-node malheuresement. Ca marche par défaut qu'avec tsc. (de ce que j'ai pu tester en tout cas ! s'il y a un moyen sans je suis preneur)

J'hésitais pour le src mais ca permettait de faire des imports depuis integration-tests/, et je trouve perso que plus clair/lisible parfois d'avoir un prefixe. Ceci dit le préfixe src n'est pas terrible, qque chose comme td-back aurait peut être plus de logique

benoitguigal · 2021-01-12T14:13:41Z

back/prisma/migrate.ts

+  port: parseInt(process.env.POSTGRES_PORT, 10)
+};
+
+migrate(dbConfig, path.join(__dirname, "migrations"))


Quel est le workflow en local en cas de conflit sur les migrations ? Par exemple, je crée une migration 5_my-migration.sql et au moment de rebase je m'aperçois que quelqu'un a déjà poussé 5_another_migration.sql. J'imagine que je dois renommer ma migration en 6_my_migration.sql mais est-ce que j'ai un moyen de rejouer 5_another_migration.sql ?

Bonne question. S'il voit 2 fichiers qui commencent par le meme nombre il bloque automatiquement et en joue aucune des deux. Mais en local, j'ai l'impression qu'on est un peu obligé de supprimer la ligne de ta migration dans la table migrations à la mano

Indeed, de même faut pas aller modifier (ne serait ce que reformater) une migration déjà passée, leur contenu est hashé et vérifié, ça plante à la suivante.

benoitguigal · 2021-01-12T14:25:23Z

back/prisma/schema.prisma

@@ -0,0 +1,506 @@
+generator client {


c'est vraiment dommage qu'ils ne permettent pas de séparer les modèles en plusieurs fichiers comme c'était le cas dans prisma 1

je crois que c'est prévu dans les futures version

pour suivi prisma/prisma#2377

benoitguigal · 2021-01-12T14:29:20Z

back/prisma/schema.prisma

+  wasteDetailsQuantity               Float?
+  wasteDetailsQuantityType           QuantityType?
+  wasteDetailsPop                    Boolean                 @default(false)
+  readableId                         String                  @unique


est-ce qu'on ajouterait pas un index sur readableId maintenant que c'est géré https://www.prisma.io/docs/concepts/components/prisma-schema/data-model#defining-an-index

La contrainte UNIQUE crée un index déjà non ?

Yep, postgre ajoute l'index sur les unique
select * from pg_indexes where tablename = 'Form';

benoitguigal · 2021-01-12T15:08:20Z

back/package.json

    "watch-graphql": "npm run graphql-codegen -- --watch",
    "start": "node src/index.js",
-    "update": "node prisma/scripts/index.js",
-    "update:dev": "ts-node prisma/scripts/index.ts",
+    "update": "node prisma/migrate.js && node prisma/scripts/index.js",


Pas certain que ce soit une bonne idée de mélanger les scripts de migration du schéma avec les scripts de migration de données. On a parfois besoin de le jouer en deux temps, par exemple pour loader les numéros SIRET de la gendarmerie il était nécessaire d'uploader un fichier sur le serveur avant.

J'avais plutot l'impression inverse: le fait que les scripts ne soient pas joués automatiquement fait qu'on a parfois oublié de les jouer lors des release (en recette notamment). Ca me paraissait pas mal de tout automatiser dans la CI, et donc d'avoir un script qui joue tout d'un coup.
Dans le cas de la gendarmerie par ex, j'aurais dit que ca a l'avantage de planter la CI au moment ou il joue le script, donc on se rend forcément compte qu'il manque le fichier et on peut relancer le truc.
Enfin je ne sais pas trop, c'est 2 approches différentes. Je ne suis pas contre modifer si vous pensez que c'est mieux

J'aime bien l'idée de le faire automatiquement, ça diminue le nombre de choses à faire manuellement (et donc les erreurs d'oubli). J'ai l'impression que le cas de la gendarmerie reste une exception. Dans 90% des cas le faire automatiquement serait ok et dans 10% des cas ça lèverait une erreur pour attirer notre attention. #my2cents

Le souci c'est qu'on a 2 systèmes de migrations, schema + data, dont l'historique de l'un est en db et l'autre géré à la mano.
(On peut jouer des fichiers js avec postgres-migrations, pas sûr que ça puisse lancer des appels prisma)

Si on garde update comme ça, faut l'avoir en tête pour les futures migrations de données qui nécessitent des étapes intermédiaires comme l'upload d'un fichier, ou les basculer dans des scripts hors migration.

Pas de préférence en ce qui me concerne.

back/integration-tests/db-deploy/deploy-db.sh

back/integration-tests/run.sh

providenz

Utilisé depuis un petit moment pour les dasris, no problem so far 👍

Riron force-pushed the mod/prisma branch 4 times, most recently from 5a85e8f to 6f7a557 Compare December 1, 2020 20:49

Riron marked this pull request as ready for review December 2, 2020 09:09

Riron requested review from zhouzi, benoitguigal and providenz December 2, 2020 09:09

Riron force-pushed the mod/prisma branch 4 times, most recently from d7ea253 to 551e4cf Compare December 8, 2020 18:46

providenz reviewed Dec 9, 2020

View reviewed changes

back/src/commands/onboarding.helpers.ts Outdated Show resolved Hide resolved

Riron force-pushed the mod/prisma branch 12 times, most recently from e8041f8 to 094b246 Compare December 17, 2020 14:24

Riron force-pushed the mod/prisma branch 4 times, most recently from e755602 to 5c94a6e Compare December 29, 2020 08:39

Riron added 14 commits January 7, 2021 06:34

Model migration

37c76bb

Use prisma2

c7d5369

Integration tests

bdb1380

Fix linting settings

dc69bf8

Unit tests

93a0e1a

Fix the rest

5f7285e

Replace subscriptions by events

8362d13

Migrate update scripts and delete prisma1

babcc6a

Add migrate script

bb0e292

Update to Prisma 2.13

2a9a9d6

Final fixes

0f06ef3

Note on migrations

1c5e900

Review

4ff24cc

Use prisma CLI instead of DB dump

b3b9169

Riron force-pushed the mod/prisma branch from e8ae543 to a2877fa Compare January 11, 2021 17:01

benoitguigal approved these changes Jan 12, 2021

View reviewed changes

benoitguigal reviewed Jan 12, 2021

View reviewed changes

back/integration-tests/db-deploy/deploy-db.sh Show resolved Hide resolved

benoitguigal reviewed Jan 12, 2021

View reviewed changes

back/integration-tests/run.sh Show resolved Hide resolved

Riron mentioned this pull request Jan 13, 2021

Validation Yup commune des draft & sealed #764

Merged

1 task

providenz approved these changes Jan 13, 2021

View reviewed changes

Riron force-pushed the mod/prisma branch 5 times, most recently from 2847469 to b3b9169 Compare January 13, 2021 16:18

Split update and migrate

76fefdd

Riron merged commit 7901301 into dev Jan 14, 2021

Riron deleted the mod/prisma branch January 14, 2021 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mod/prisma #733

Mod/prisma #733

Riron commented Nov 30, 2020 •

edited

Loading

providenz commented Dec 9, 2020

Riron commented Jan 11, 2021

benoitguigal left a comment

benoitguigal Jan 12, 2021

benoitguigal Jan 12, 2021

Riron Jan 13, 2021

zhouzi Jan 13, 2021

Riron Jan 13, 2021 •

edited

Loading

benoitguigal Jan 12, 2021

Riron Jan 13, 2021

providenz Jan 13, 2021

benoitguigal Jan 12, 2021

Riron Jan 13, 2021

Riron Jan 13, 2021

benoitguigal Jan 12, 2021

Riron Jan 13, 2021

providenz Jan 13, 2021

benoitguigal Jan 12, 2021

Riron Jan 13, 2021

zhouzi Jan 13, 2021

providenz Jan 13, 2021

providenz left a comment

Mod/prisma #733

Mod/prisma #733

Conversation

Riron commented Nov 30, 2020 • edited Loading

Code

DB

Test d'inté

Migrations

providenz commented Dec 9, 2020

Riron commented Jan 11, 2021

benoitguigal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Riron Jan 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

providenz left a comment

Choose a reason for hiding this comment

Riron commented Nov 30, 2020 •

edited

Loading

Riron Jan 13, 2021 •

edited

Loading