Consolidation BNLC : détecte les erreurs lors du décodage d'un CSV

[AntoineAugusti]

Traite le problème détaillé ici #3419 (comment)

Certains fichiers CSV peuvent être invalides car mal encodés, le cas vu était la présence d'un " au sein d'un champ sans être échappé.

[AntoineAugusti]

Je suis tombé sur un problème avec csv dans mon premier commit, ça m'a pris du temps de remonter le problème beatrichartz/csv#125

[AntoineAugusti]

C'est du boy scout ceci et le renommage de :validation_error vers :validator_unavailable_error

[AntoineAugusti]

@AurelienC Avec ce changement tu pourras conserver Châlon dans datasets.csv et ce sera indiqué qu'il y a bien une erreur lors du décodage.

[vdegove]

Ça me semble bien à part une micro typo dans un commentaire et une question de nommage.

Si je comprends bien, vu ton commentaire sur CSV.decode!, à la base, tu voulais gérer ça directement dans consolidate_resources (la partie append other valid resources) ? Ou pas ? En tout cas ça fait deux passes de décodage de CSV des resources d’origine, une pour vérifier la bonne conformité, l’autre pour append au fichier consolidé, donc ça rajoute un peu de temps de traitement, mais j’imagine que c’est mineur et qu’il n’y a pas de solution en une passe ?

[AntoineAugusti]

@vdegove Merci pour ta review, j'ai traité tes commentaires !

Si je comprends bien, vu ton commentaire sur CSV.decode!, à la base, tu voulais gérer ça directement dans consolidate_resources (la partie append other valid resources) ?

Non je voulais traiter avant consolidate_resources. Cette fonction prend normalement des choses qui sont conformes et validées et peut ensuite dérouler (lire des fichiers valides, déjà présents sur le disque, bon format etc).

transport-site/apps/transport/lib/jobs/consolidate_bnlc_job.ex

Lines 66 to 76 in 54bc471

	consolidate_resources(download_details)
	Logger.info("Sending the email recap")
	upload_temporary_file()
	|> schedule_deletion()
	|> send_email_recap(%{
	dataset_errors: dataset_errors,
	validation_errors: validation_errors,
	download_errors: download_errors
	})

En tout cas ça fait deux passes de décodage de CSV des resources d’origine, une pour vérifier la bonne conformité, l’autre pour append au fichier consolidé, donc ça rajoute un peu de temps de traitement, mais j’imagine que c’est mineur et qu’il n’y a pas de solution en une passe ?

En fait ce n'était pas prévu que cette erreur arrive. Aurélien et moi avons été surpris qu'une telle erreur (unescaped double quotes in a value) passe le validateur TableSchema et on a eu un crash inattendu dans CSV.decode! dans consolidate_resources. On a donc cherché à détecter cette erreur potentielle en amont.

Je pense qu'il est possible d'avoir un seul décodage, mais pas sûr que ça vaille le coup du refactor car :

• CSV.decode est fait en streaming et on veut minimiser l'usage mémoire de manière générale (on aurait pu passer tous les contenus de fichiers en mémoire mais bof)
• la BNLC va faire < 10k lignes ce qui n'est pas énorme si on doit décoder 2 fois le tout en CSV
• le temps d'exécution, tant qu'il est inférieur à ~30s est tout à fait acceptable au vu du travail réaliser : consolider un fichier d'une base nationale. Exécution 1 fois par jour en rythme de production, moins actuellement. On est très loin de ce temps actuellement, on est sur du 5s de mémoire