Refacto consolidation IRVE : un seul passage fichier vers dataframe et vérification pré-validation

[vdegove]

(Dernière ?) grosse PR de plomberie sur la consolidation pour se défaire de raccourcis d’architectures pris lors de l’implémentation initiale. Pas d’impact produit sauf une amélioration de la vélocité de la consolidation : pour tous les fichiers déjà présents (même checksum) en base de données, la validation n’est plus réalisée, on a désormais une vérification court-circuit de ce qui est déjà présent en base.

Recasting du dataframe pré-insertion en base

Fixes #5232

On avait jusqu’à présent un traitement séparé de la resource pour la validation et de l’insertion en base. On prenait la resource, on la castait en dataframe non typé (tout en string), puis on validait, puis on repartait à nouveau de la resource sur le disque pour la caster en dataframe typé, qui lui était inséré en base.

Maintenant, la validation passe son dataframe non typé à l’insertion en base, qui est capable de caster les types, puis enfin insérer tout ça proprement.

Cela permet de se débarrasser de pas mal de code un peu similaire qui était franchement casse gueule, puisqu’on supprime la fonction read_as_data_frame(body) en doublon de read_as_uncasted_dataframe(body) ; et qu’on avait à gauche à droite des options type=strict ou pas dans les fonctions plus profondes de traitement.

Avant :
resource disque -> read_as_uncasted_dataframe -> validation
PUIS
resource disque -> read_as_data_frame -> insertion en base.

Maintenant :
resource disque -> read_as_uncasted_dataframe -> validation -> cast_validated_frame -> insertion en base.

Bon forcément ça fait un gros diff, j’ai fait de mon mieux mais ça va être un peu ardu à lire.

Court-circuit de la validation si fichier déjà en base de données

On calcule désormais le checksum avant toute chose lors du traitement de la resource, et si on voit qu’il est identique à une version en base déjà validée, on arrête le traitement, ce qui permet de s’économiser une validation et une tentative d’insertion en base rejetée par PostgreSQL.

Point d’attention : réécriture d’une partie de la base lors du premier run

La définition du checksum change : jusqu’à maintenant il s’agissait du checksum post-transcodage en UTF-8 de la resource, à présent il s’agit du fichier brut tel que téléchargé.

Cela signifie que pour une partie de la base (normalement, que celle concernée par le transcodage), on va réécrire le fichier et ses PDC à l’identique, donc ce qui veut dire un traitement plus lourd.

Et l’expérience montre que la production n’aime pas beaucoup quand trop de resources sont à écrire en base de données en même temps.

Bon en pratique, avec un test local, l’essentiel des resources est bien détecté comme :already_in_db et donc n’est pas réinséré :

iex(7)> df["status"] |> Explorer.Series.frequencies() |> Explorer.DataFrame.print(limit: 20)
+-------------------------------------------+
| Explorer DataFrame: [rows: 6, columns: 2] |
+--------------------------------+----------+
|             values             |  counts  |
|            <string>            |  <u32>   |
+================================+==========+
| already_in_db                  | 1279     |
| producer_not_an_organization   | 188      |
| file_level_errors              | 147      |
| import_successful              | 104      |
| download_failed                | 100      |
| not_compliant_with_schema      | 33       |
+--------------------------------+----------+

Mais à surveiller quand même.

[thbar]

La méthode try_write_uncasted_df/3 (j'aimais bien le nom et ça semblait raccord avec le reste du code) n'existe pas.

Du coup est ce qu'on fait:

Suggested change

	There are two ways to use this module, either through `try_write_uncasted_df/3` (entry point for the consolidation)
	There are two ways to use this module, either through `try_write_untyped_df/3` (entry point for the consolidation)

ou est-ce qu'on renomme la méthode ?

[thbar]

Je propose du renommage de la méthode, ça paraît plus aligné avec le vocabulaire, mais au final y'a deux options:

1. Correction du commentaire
2. Renommage de la méthode !

[thbar]

Est-ce que je comprends bien (à voir le changement ligne 333) et on a plutôt ça ?

Ou est-ce que ça dépend ?

Suggested change

	with empty string values.

EDIT: le rendering de GitHub sur ma suggestion a l'air bizarre...

[thbar]

Suggested change

	import_status <- Transport.IRVE.DatabaseImporter.try_write_untyped_df(validated_df, checksum, resource) do
	import_status <- Transport.IRVE.DatabaseImporter.try_write_uncasted_df(validated_df, checksum, resource) do

[thbar]

Suggested change

	def try_write_untyped_df(untyped_df, checksum, %{
	def try_write_uncasted_df(untyped_df, checksum, %{

[thbar]

Idem, à voir si on corrige le moduledoc, ou on change le nom de la méthode, comme tu préfères.

[thbar]

Suggestion: repousser la lecture et le calcul à après la vérification du fait que le producteur est une org.

Quelque chose comme (à adapter):

with :producer_is_an_organization <- producer_is_org(resource),
  body = File.read!(path),
  checksum = Transport.IRVE.DatabaseImporter.compute_checksum(body),

Pas strictement nécessaire, mais si on part du principe qu'un individuel peut faire + de problèmes, peut-être utile ;-)