Ajout validateur GBFS à la multi-validation #2554

AntoineAugusti · 2022-07-28T14:18:33Z

En lien avec #2390 #2555

Cette PR ajoute la validation GBFS au processus de multi-validation.

Elle comporte :

l'ajout du validateur dans transport
un job de dispatch pour toutes les ressources GBFS, et pour effectuer et enregistrer une multi-validation pour une ressource GBFS
une exécution automatique quotidienne
les tests associés

Ce qui manque :

brancher le web sur la multi-validation pour le GBFS
supprimer l'ancienne classe de validation
utiliser les métadonnées dans resource_metadata

Je découpe ainsi pour ne pas avoir trop de code et pour avoir un peu d'historique à disposition en base de données.

thbar

J'ai fait une première passe "ultra rapide" à l'instant.

Est-ce que tu sais si le validateur hébergé est effectivement proche de master (cf discussion sur MobilityData/gbfs-validator#77), car il y aurait un auto-deploy par exemple ?

Je suis toujours méfiant de stocker une info de ce type si elle n'est pas garantie, car ça peut faire perdre pas mal de temps derrière.

Cela étant dit j'irai reviewer plus en détail et faire tourner en local pour prendre la main sur le sujet, et je reviens vers toi ! (sauf si Francis l'a fait avant, vu que je vais être en congés par tranches).

Beau boulot par ailleurs et par avance, c'est cool de voir la multi-validation se déployer sur davantage de formats !

AntoineAugusti · 2022-08-18T09:19:37Z

@thbar Je suis quasiment certain que gbfs-validator est déployé automatiquement sur master avec Netlify ! J'ai fait quelques PRs là-bas et ça passait en production en quelques minutes. C'était il y a quelques mois mais je ne pense pas que ceci ait changé. Je vais demander pour vérification.

PierrickP · 2022-08-18T11:55:55Z

Bonjour @AntoineAugusti @thbar , je vous confirme que gbfs-validator est déployé automatique au commit sur master (ca utilise Netlify)
On doit pouvoir rajouter le sha du commit git à la réponse d'API.

Pour que je comprenne bien, quelle serait l'usage pour vous de cette information ?

thbar · 2022-08-18T12:06:53Z

Hello @PierrickP! Merci pour pour la confirmation!

Pour que je comprenne bien, quelle serait l'usage pour vous de cette information ?

Cela permet de noter la version précise du validateur contre lequel un rapport de validation aura été calculé, dans le but par exemple de pouvoir reproduire localement un rapport avec la bonne version du validateur, quand on débuggue quelque chose.

thbar

J'ai pu faire tourner le code en local comme suit:

[resource_id] = Transport.Jobs.GBFSMultiValidationDispatcherJob.relevant_resources()
|> Enum.drop(3)
|> Enum.take(1)

Transport.Jobs.GBFSMultiValidationJob.perform(%Oban.Job{args: %{"resource_id" => resource_id}})

J'ai eu des résultats au début négatifs, je partage mes copies d'écran:

Du coup je me suis demandé:

pourquoi on a ces erreurs
est-ce lié au statut "hébergé" du validateur
comment les reproduire en local pour comprendre

J'ai vu passer les discussions suivantes:

Avec notamment:

The validator is currently hosted on my personnal Netlify (free plan, limited to 125k requests/month) account and we are in the way to transfer it to MobilityData.

J'ai pu reproduire au final les erreurs:

❯ node cli.js "https://gateway.prod.zoov.io/gbfs/2.2/bordeaux/en/station_information.json?key=NGFlMjU3MDUtNDk5My00MTM4LTk1ZjctNmNlNDM1MWQ0NjE1"
/Users/thbar/git/transport/gbfs-validator/gbfs-validator/gbfs.js:314
          this.autoDiscovery.data[key[0]].feeds.find(f => f.name === type)
                                                ^

TypeError: Cannot read properties of undefined (reading 'find')
    at /Users/thbar/git/transport/gbfs-validator/gbfs-validator/gbfs.js:314:49
    at Array.map (<anonymous>)
    at GBFS.getFile (/Users/thbar/git/transport/gbfs-validator/gbfs-validator/gbfs.js:311:60)
    at /Users/thbar/git/transport/gbfs-validator/gbfs-validator/gbfs.js:437:27
    at Array.map (<anonymous>)
    at GBFS.validation (/Users/thbar/git/transport/gbfs-validator/gbfs-validator/gbfs.js:437:13)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

Plus important: celle qui donne un timeout marche en local, qui est probablement lié à Netlify.

Conclusion: j'approuve en l'état, ça me semble bon ! Toutefois je recommande qu'on surveille ce qui se passe, et qu'on passe en CLI si on constate trop de soucis.

PierrickP · 2022-08-22T07:20:08Z

@thbar Pour Vélib, ils bloquent les ips de datacenter (avec cloudflare). Mis à part faire en local ou utiliser des proxies, pas de solution

Pour Zoov: (attention à la key, c'est peut-etre personnel ?), si l'on passe l'url du gbfs.json et non station_information.json, la validation est bonne.
Je vais faire une issue pour mieux detecter une url "invalide"

AntoineAugusti · 2022-08-22T07:22:51Z

@PierrickP Faut qu'on regarde si un tel filtrage n'est pas contraire à la loi 😉

AntoineAugusti · 2022-08-22T08:12:42Z

@PierrickP La clé est bien publique ici

AntoineAugusti · 2022-08-22T08:45:35Z

@etalab/transport-tech Dans mon dernier commit 49f5551, j'ai remplacé le code utilisé pour avoir la version du validateur GBFS en utilisant l'API GitHub par du code lisant cette information dans le rapport d'erreur du validateur GBFS.

MobilityData/gbfs-validator#78 n'est pas encore passée en prod, il faut attendre un peu.

@fchabouis tu me diras si tu veux relire avant que je merge (ou pas !)

AntoineAugusti · 2022-08-14T12:26:24Z

apps/transport/lib/validators/gbfs_validator.ex

+  @behaviour Transport.Validators.Validator
+
+  @impl Transport.Validators.Validator
+  def validate_and_save(%DB.Resource{url: url, format: "gbfs", id: resource_id}) do


Prend une ressource en param et non une ResourceHistory car les ressources GBFS ne sont pas historisées (car c'est du temps réel).

AntoineAugusti · 2022-08-14T12:27:12Z

apps/transport/lib/validators/gbfs_validator.ex

+
+  @impl Transport.Validators.Validator
+  def validate_and_save(%DB.Resource{url: url, format: "gbfs", id: resource_id}) do
+    result = GBFSMetadata.compute_feed_metadata(url, "https://#{Application.fetch_env!(:transport, :domain_name)}")


Ça prend l'adresse du site web pour déterminer si le PAN est autorisé via CORS. À voir si c'est vraiment utile ou si en réalité on l'utilise pas, c'est dans la classe existante.

AntoineAugusti · 2022-08-14T12:29:17Z

apps/transport/test/transport/validators/gbfs_validator_test.exs

+    Transport.Shared.GBFSMetadata.Mock
+    |> expect(:compute_feed_metadata, fn ^url, "https://transport.data.gouv.fr" ->


On pourrait peut-être simplifier les tests, là c'est du copié collé dans le test du job par rapport au test du validateur.

Peut-être lors de la suppression de la classe de validation dans shared en mettant un behaviour ?

AntoineAugusti · 2022-08-14T12:29:38Z

apps/transport/test/transport/validators/gbfs_validator_test.exs

+           } = DB.MultiValidation |> DB.Repo.one!() |> DB.Repo.preload(:metadata)
+  end
+
+  defp setup_validator_version_mocks(default_branch \\ "master", sha \\ Ecto.UUID.generate()) do


Même remarque qu'avant, j'ai repris les tests du validateur.

AntoineAugusti · 2022-08-14T12:30:07Z

config/runtime.exs

@@ -98,6 +98,7 @@ oban_crontab_all_envs =
        {"30 */6 * * *", Transport.Jobs.BNLCToGeoData},
        {"15 10 * * *", Transport.Jobs.DatabaseBackupReplicationJob},
        {"0 7 * * *", Transport.Jobs.GTFSRTMultiValidationDispatcherJob},
+        {"30 7 * * *", Transport.Jobs.GBFSMultiValidationDispatcherJob},


30mn plus tard que la validation GTFS-RT. Dites-moi ce que vous en pensez en terme de fréquence.

je pense qu'on va adapter par la suite si la cette fréquence pose problème.

AntoineAugusti · 2022-08-22T08:46:57Z

gosh, j'avais pas posté tous mes commentaires en attente 😅 désolé @thbar

AntoineAugusti · 2022-08-22T11:28:37Z

PR ajoutant la version du validateur mergée ! On peut passer ça en prod quand on veut.

fchabouis · 2022-08-22T11:58:23Z

je vais essayer de relire dans l'aprem, on peut merger ce soir ?

AntoineAugusti · 2022-08-22T13:27:31Z

Sans problème, j'attends :)

fchabouis · 2022-08-22T14:57:56Z

tout bon pour moi !

AntoineAugusti added 5 commits July 27, 2022 15:02

Add class

18c7b7b

Work on test

beacefb

I can make credo happy

b870479

Validate command

db9a862

Update doc

4e5c663

AntoineAugusti mentioned this pull request Jul 28, 2022

Multi-validation pour le GBFS #2555

Closed

AntoineAugusti added 2 commits August 14, 2022 05:50

Merge branch 'master' into transport_gbfs_validator

52a7292

Add GBFSMultiValidationDispatcherJob

7463471

AntoineAugusti marked this pull request as ready for review August 14, 2022 12:33

AntoineAugusti requested a review from a team as a code owner August 14, 2022 12:33

Merge branch 'master' into transport_gbfs_validator

02b5b6d

thbar reviewed Aug 17, 2022

View reviewed changes

AntoineAugusti mentioned this pull request Aug 18, 2022

Output gbfs-validator version in the HTTP API response MobilityData/gbfs-validator#77

Closed

thbar approved these changes Aug 19, 2022

View reviewed changes

Read validator version from summary

49f5551

PierrickP mentioned this pull request Aug 22, 2022

Better detection of invalid url MobilityData/gbfs-validator#79

Open

AntoineAugusti commented Aug 22, 2022

View reviewed changes

Merge branch 'master' into transport_gbfs_validator

ec31f99

AntoineAugusti merged commit d952571 into master Aug 23, 2022

AntoineAugusti deleted the transport_gbfs_validator branch August 23, 2022 07:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ajout validateur GBFS à la multi-validation #2554

Ajout validateur GBFS à la multi-validation #2554

AntoineAugusti commented Jul 28, 2022 •

edited

Loading

thbar left a comment

AntoineAugusti commented Aug 18, 2022

PierrickP commented Aug 18, 2022

thbar commented Aug 18, 2022

thbar left a comment

PierrickP commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

AntoineAugusti Aug 14, 2022

AntoineAugusti Aug 14, 2022

AntoineAugusti Aug 14, 2022

AntoineAugusti Aug 14, 2022

AntoineAugusti Aug 14, 2022

fchabouis Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

fchabouis commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

fchabouis commented Aug 22, 2022

		Transport.Shared.GBFSMetadata.Mock
		\|> expect(:compute_feed_metadata, fn ^url, "https://transport.data.gouv.fr" ->

Ajout validateur GBFS à la multi-validation #2554

Ajout validateur GBFS à la multi-validation #2554

Conversation

AntoineAugusti commented Jul 28, 2022 • edited Loading

thbar left a comment

Choose a reason for hiding this comment

AntoineAugusti commented Aug 18, 2022

PierrickP commented Aug 18, 2022

thbar commented Aug 18, 2022

thbar left a comment

Choose a reason for hiding this comment

PierrickP commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

AntoineAugusti Aug 14, 2022

Choose a reason for hiding this comment

AntoineAugusti Aug 14, 2022

Choose a reason for hiding this comment

AntoineAugusti Aug 14, 2022

Choose a reason for hiding this comment

AntoineAugusti Aug 14, 2022

Choose a reason for hiding this comment

AntoineAugusti Aug 14, 2022

Choose a reason for hiding this comment

fchabouis Aug 22, 2022

Choose a reason for hiding this comment

AntoineAugusti commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

fchabouis commented Aug 22, 2022

AntoineAugusti commented Aug 22, 2022

fchabouis commented Aug 22, 2022

AntoineAugusti commented Jul 28, 2022 •

edited

Loading