etalab / transport-site

Rendre disponible, valoriser et améliorer les données transports
https://transport.data.gouv.fr
190 stars 29 forks source link

Bug de moissonnage des données de la région SUD #1564

Closed thbar closed 3 years ago

thbar commented 3 years ago

Remonté par @NicolasBerthelot:

Pour tous les datasets on arrive bien à télécharger le GTFS, on en fait la validation, mais on indique qu'il n'est pas disponible. Je ne comprends vraiment pas quelle peut être l'origine du problème. Les datasets concernés :

thbar commented 3 years ago

En visuel cela donne:

CleanShot 2021-03-30 at 09 33 15@2x
Miryad3108 commented 3 years ago

C'est également le cas pour ces ressources https://transport.data.gouv.fr/datasets/reseau-de-transport-en-commun-de-la-communaute-agglomeration-ventoux-comtat-venaissin/

https://transport.data.gouv.fr/datasets/reseau-de-transport-urbain-agglobus-cavem/

thbar commented 3 years ago

@fchabouis a suggéré qu'on considère temporairement que "tout est disponible", car globalement c'est le cas, et l'évaluation de la disponibilité n'est pas fiable et donne une fausse impression d'indisponibilité.

Et on retravaillera ensuite sur une v2 de disponibilité plus fiable comme évoqué ensemble.

Qu'en pensez-vous ?

Miryad3108 commented 3 years ago

Je suis d'avis de le garder car parfois c'est "non disponible" parce que la ressource a été supprimée soit sur data.gouv.fr soit sur le portail OpenData (ou l'URL a changé). Quand la ressource a été supprimée sur data.gouv.fr on peut le savoir via les mails automatiques qu'on reçoit. Par contre, quand la ressource a été supprimée sur le portail de la collectivité, on a que cette étiquette pour nous alerter.

NicolasBerthelot commented 3 years ago

Assez d'accord avec @Miryad3108, en revanche ce serait vraiment idéal de pouvoir mettre une exception pour les régions issus de data sud temporairement, c'est vraiment dommage de considérer qu'ils sont indisponibles. Mais si c'est impossible de faire au cas par cas, alors tant pis, je suis de l'avis de Miryad.

thbar commented 3 years ago

@NicolasBerthelot je vais aller débugger ce cas là, c'est vrai que ça fait plusieurs cas.

thbar commented 3 years ago

Reproduction:

$ hex -S mix

iex(2)> url = "https://trouver.datasud.fr/dataset/e8c44069-1822-4778-908d-05d699bfe826/resource/195f6924-ffb9-4704-9975-0c98abc7aa0c/download/gtfs_cp.zip"

iex(3)> HTTPoison.get(url)
[info] TLS :client: In state :certify at ssl_handshake.erl:1783 generated CLIENT ALERT: Fatal - Handshake Failure
 - {:bad_cert, :invalid_key_usage}
{:error,
 %HTTPoison.Error{
   id: nil,
   reason: {:tls_alert,
    {:handshake_failure,
     'TLS client: In state certify at ssl_handshake.erl:1783 generated CLIENT ALERT: Fatal - Handshake Failure\n {bad_cert,invalid_key_usage}'}}
 }}
thbar commented 3 years ago

Je tente différents trucs, je ferai un rapport ici quand j'aurai trouvé.

thbar commented 3 years ago

Statistiques pour voir combien de ressources sont impactés (avant fix) - c'est la ligne tls_alert:

result Count - url
200 1801
301 61
302 7
307 1
404 266
405 58
503 1
:timeout 10
{:tls_alert, {:handshake_failure, 'TLS client: In state certify at ssl_handshake.erl:1783 generated CLIENT ALERT: Fatal - Handshake Failure\n {bad_cert,invalid_key_usage}'}} 15
Total Result 2220

EDIT: Je vois que 15 ressources sont concernées, et elles sont toutes sur le serveur trouver.datasud.fr.

thbar commented 3 years ago

Code utilisé pour créer le fichier CSV (après j'ai fait un tableau croisé dynamique):

{:ok, _} = Application.ensure_all_started(:hackney)
{:ok, _} = Application.ensure_all_started(:ecto)
{:ok, _} = Application.ensure_all_started(:db)

import Ecto.Query

DB.Resource
#|> limit(2)
|> DB.Repo.all
|> Enum.map(&(&1.url))
|> Enum.reject(&(URI.parse(&1).scheme == "ftp"))
|> Task.async_stream(
  fn(url) ->
    result = case r = HTTPoison.head(url) do
      {:error, e} -> e.reason
      {:ok, e} -> e.status_code
    end
    {url, result}
  end,
  timeout: 15_000,
  on_timeout: :kill_task,
  max_concurrency: 50
)
|> Enum.to_list()
|> Enum.map(fn({:ok, {url, res}}) -> [url, res |> inspect] end)
|> CSV.encode
|> Enum.to_list()
|> IO.puts
thbar commented 3 years ago

La solution propre serait de mettre à jour OTP comme je l'ai constaté (#1584), mais c'est un trop gros changement. Je vais préparer un hotfix, toutefois il faudra voir si il faut l'ajouter à d'autres endroits, car le code fait appel à HTTPoison ailleurs.

thbar commented 3 years ago

En remplaçant l'appel:

HTTPoison.head(url)

par:

HTTPoison.head(url, [], ssl: [versions: [ :'tlsv1.2' ]] )

J'obtiens les statistiques suivantes:

result Count - url
200 1815
301 61
302 7
307 1
404 267
405 58
503 1
:timeout 10
Total Result 2220
thbar commented 3 years ago

@NicolasBerthelot j'ai implémenté un fix temporaire dans #1585. Ca semble fonctionner, la plupart des ressources passent en vert, sauf une que j'ai vu passer en "illisible" mais ça sera un autre souci.

Je te laisse vérifier si tu peux et fermer ce ticket si c'est bon pour toi.

thbar commented 3 years ago

@NicolasBerthelot je ferme le ticket à présent, ça semble OK.