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

Correction de la détection des ressources non disponibles #1666

Merged
merged 10 commits into from
Jun 7, 2021

Conversation

fchabouis
Copy link
Contributor

closes #1665
closes #1538

L'idée est de supprimer tous les cas particuliers et de traiter toutes les ressources de la même manière :

  • on fait une requête head, en suivant les éventuelles redirections pour voir si la ressources est bien disponible
  • en cas de réponse 405 (signifiant que la méthode HEAD n'est pas supportée), on passe au plan B : on fait un GET en streamant la réponse. Dès qu'on a l'information sur le statut de la réponse, on arrête le streaming pour gagner du temps.

Question à laquelle je viens de penser : quid des ressources sur ftp ? (voir #1409)

@thbar
Copy link
Contributor

thbar commented Jun 7, 2021

@fchabouis pour les ressources FTP, j'ai un plan qui est de systématiquement passer par le proxy Elixir pour créer ces ressources, ce qui fera qu'on n'aura pas de cas particulier à gérer (on peut en reparler de vive voix si ce n'est pas clair), ça se fera en HTTPS.

@fchabouis fchabouis requested a review from thbar June 7, 2021 12:27
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.

Quelques petites choses à améliorer idéalement, et je me demande si tu as pu faire un test global sur toutes les ressources pour voir comment ça se comporte aussi !

apps/shared/lib/http_stream_v2.ex Outdated Show resolved Hide resolved
apps/shared/test/http_stream_v2_test.exs Outdated Show resolved Hide resolved
apps/shared/test/http_stream_v2_test.exs Outdated Show resolved Hide resolved
apps/transport/lib/transport/availability_checker.ex Outdated Show resolved Hide resolved
@fchabouis
Copy link
Contributor Author

J'ai fait tourner sur l'ensemble des 2304 ressources. On a 294 ressources non dispo et 2010 disponibles.
Et pas de crash :)

iex(6)> urls = DB.Resource |> DB.Repo.all() |> Enum.map(fn r -> r.url end)
iex(7)> urls |> Enum.count()                       
2304
iex(8)> available = urls |> Enum.map(fn u -> Transport.AvailabilityChecker.available?(u) end)
iex(9)> available |> Enum.frequencies()                                   
%{false: 294, true: 2010}

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.

Ca paraît good ! Beau boulot !

@fchabouis fchabouis merged commit 1189944 into master Jun 7, 2021
@fchabouis fchabouis deleted the fix-available-function branch June 7, 2021 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants