etalab / gtfs_converter

Convert files of transport.data.gouv.fr into other formats
4 stars 3 forks source link

fix the subprocess handing #24

Closed antoine-de closed 4 years ago

antoine-de commented 4 years ago

I dont know what I'm doiiiiiiiing

the subprocess can hang when the buffers are full (https://thraxil.org/users/anders/posts/2008/03/13/Subprocess-Hanging-PIPE-is-your-enemy/)

There is a warning in the doc for wait() (https://docs.python.org/3/library/subprocess.html#subprocess.Popen.wait) (we don't use it directly, but maybe it is used by poll ?), telling us to use communicate, but I don't see how I can use it and still have streamed log.

SO I went back to the good old stack overflow copypasting, using select to pick the data without deadlock. I don't understand half the copy pasted lines, but seems to work...

fchabouis commented 4 years ago

Je me demande : on tient vraiment à streamer la sortie ?

Pour le call synchrone, j'utilise juste subprocess.check_output(command), mais je loupe peut-être quelque chose.

Sinon, c'est vrai que le code est un peu cryptique comme ça :)

antoine-de commented 4 years ago

ben c'est pratique, les traitements peuvent être un peu long, c'est dommage de ne rien voir, pis d'avoir des logs au bout de 10mn non ? Ca rendrait le debug plus difficile.

fchabouis commented 4 years ago

Ça ne mettrait pas les logs du convertisseur gtfs2netex (code rust) au fur et à mesure, mais tout le reste (code python) arriverait quand même au fil de l'eau. Je ne sais pas si c'est utile que les logs du convertisseur soit real time ? Normalement c'est pas lui qui plante ;) J'avoue que je ne comprends pas trop le code, c'est pour ça que je suis tenté de revenir à une solution plus simple... Après si tu veux tenter comme ça et voir si ça marche, pourquoi pas.

antoine-de commented 4 years ago

ben même si ca plante pas, ce sont des process un poil long, j'ai peur que ca soit relou de ne rien voir bouger pendant 10mn quand on fait tourner un truc.