Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ajout client pour l'API de conversion EnRoute #3542

Merged
merged 12 commits into from
Oct 19, 2023

Conversation

AntoineAugusti
Copy link
Member

@AntoineAugusti AntoineAugusti commented Oct 17, 2023

Crée Transport.Converters.GTFSToNeTExEnRoute pour utiliser l'API de conversion GTFS vers NeTEx d'EnRoute :

  • créer une conversion
  • vérifier l'état d'avancement de la conversion
  • télécharger une conversion

ceci est accompagné de la configuration et de tests. C'est l'occasion d'utiliser Req dans notre codebase pour faire du téléchargement en streaming de réponses HTTP. Merci @thbar pour ton aide.

La suite des tâches à faire est dans #3427

@AntoineAugusti
Copy link
Member Author

@thbar Ça y est j'ai #3499 sur cette branche en cherchant à mettre req. Une idée ? Je vais prendre le fork de phoenix_markdown mais j'ai eu une erreur aussi en CI il me semble.

@thbar
Copy link
Contributor

thbar commented Oct 17, 2023

@AntoineAugusti oui il faut forker et intégrer boydm/phoenix_markdown#13 dans notre fork (dans etalab plutôt qu'en perso si possible !)

@thbar
Copy link
Contributor

thbar commented Oct 17, 2023

(en tout cas ça devrait fonctionner, à vérifier !)

@AntoineAugusti AntoineAugusti marked this pull request as ready for review October 17, 2023 14:37
@AntoineAugusti AntoineAugusti requested a review from a team as a code owner October 17, 2023 14:37
@AntoineAugusti
Copy link
Member Author

Navré pour le bruit avec ces commits, ça devrait être bon après des fixs sur phoenix_markdown et scrivener_html (forké sur Etalab)

config/config.exs Outdated Show resolved Hide resolved
@AntoineAugusti
Copy link
Member Author

@thbar Elle est OK à review si tu veux. L'utilisation de Bypass est discutable, je suis d'accord. Je me dis que tant que c'est dans ce fichier ça va, après j'imagine qu'on aura un wrapper HTTP + behaviour et donc pas besoin de faire ça.

@thbar
Copy link
Contributor

thbar commented Oct 19, 2023

L'utilisation de Bypass est discutable, je suis d'accord. Je me dis que tant que c'est dans ce fichier ça va, après j'imagine qu'on aura un wrapper HTTP + behaviour et donc pas besoin de faire ça.

"discutable" c'est sévère 😄 disons que je trouve ça très bien dès lors que tu veux évaluer réellement le stack HTTP jusqu'au bout parce qu'il y a un risque de fragilité dessus (comme 0720c4a) ou que l'implémentation du composant est assez "bas niveau" (comme dans https://github.com/etalab/transport-site/blob/master/apps/shared/test/http_stream_v2_test.exs) ; je partage les liens davantage pour @vdegove plus que pour toi !

Même si ça marche très bien en Elixir (pour @vdegove, bypass démarre un serveur/plug sur un autre port dans le même process parce qu'on est en Elixir et que Yolo ça passe), ça crée un peu de lenteur petit à petit si on s'en sert trop etc (comparativement à du mock qui ne réalise par réellement la requête), mais c'est très pratique quand besoin !

Mais 💯 d'accord ça ne pose aucun souci et on adaptera après quand on aura le bon wrapper !

@AntoineAugusti AntoineAugusti requested a review from thbar October 19, 2023 08:51
@AntoineAugusti
Copy link
Member Author

@thbar Me semble que ton dernier commentaire était globalement une approbation. Ok pour ✅ ?

@thbar
Copy link
Contributor

thbar commented Oct 19, 2023

@AntoineAugusti nope! Je fais un petit test en local pour m'approprier et je reviens vers toi rapidement.

@AntoineAugusti AntoineAugusti linked an issue Oct 19, 2023 that may be closed by this pull request
Copy link
Contributor

@thbar thbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On est bon, j'ai pu tester en local pour m'approprier la chose (merci @AntoineAugusti de ta patience).

Voici le script (poke @vdegove ça peut te parler aussi pour appréhender l'API):

key = "XYZ"

Application.put_env(:transport, :enroute_token, key)

# https://transport.data.gouv.fr/datasets/reseau-urbain-et-interurbain-dile-de-france-mobilites
IO.puts("Creating conversion...")
file = "gtfs-diviamobilites-current.zip"
# file = "IDFM-gtfs.zip"
uuid = Transport.Converters.GTFSToNeTExEnRoute.create_gtfs_to_netex_conversion(file)
IO.puts("uuid=#{uuid}")

Stream.unfold(1, fn
  acc ->
    :timer.sleep(1_000)

    case Transport.Converters.GTFSToNeTExEnRoute.get_conversion(uuid) do
      {:pending, json} ->
        IO.puts("Conversion pending: #{json |> inspect}")
        {acc, acc + 1}

      {:failed, json} ->
        IO.puts("Conversion failed! #{json |> inspect}")
        nil

      {:success, json} ->
        IO.puts("Conversion succeeded! #{json |> inspect}")
        IO.puts("Downloading...")
        :ok = Transport.Converters.GTFSToNeTExEnRoute.download_conversion(uuid, File.stream!("netex.zip"))
        IO.puts("File ready! ")
        nil
    end
end)
|> Stream.run()

Je constate un temps de traitement de 45 minutes sur le GTFS d'IDFM:

%{"ended_at" => "2023-10-19 11:58:27 +0200", "id" => "0a608dc5-3ffa-4153-b9e9-be4db4936809", "options" => %{"profile" => "french"}, "started_at" => "2023-10-19 11:14:20 +0200", "status" => "success", "type" => "gtfs-netex"}

Et beaucoup plus rapide sur Dijon:

%{"ended_at" => "2023-10-19 11:57:42 +0200", "id" => "5d95f596-4f55-4bc1-bbbc-c3445db243da", "options" => %{"profile" => "french"}, "started_at" => "2023-10-19 11:55:57 +0200", "status" => "success", "type" => "gtfs-netex"}

Les temps de traitement pourront avoir un impact sur le design de la suite d'ailleurs !

Beau boulot @AntoineAugusti !

@AntoineAugusti AntoineAugusti added this pull request to the merge queue Oct 19, 2023
Merged via the queue into master with commit 73c9b8e Oct 19, 2023
@AntoineAugusti AntoineAugusti deleted the converter_client_enroute branch October 19, 2023 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug de compilation sur phoenix_markdown
2 participants