Produits Entr'ouvert » Hobo

Hum, "améliorer" c'est vraiment trop vague. Et en l'espèce c'est vague parce qu'il y a des tonnes de trucs différents, dont des passages sur lesquels je m'interroge lourdement, genre .replace('https','http'), ou hobo_json = sys.stdin.readlines()[0].

Oui je reconnais que c'est assez vague.

Reste à séparer le commit et à associer des messages corrects. (un pourrait concerner le logging, un autre la création et l'utilisation d'un utilisateur "hobo-piwik").

Concernant le self.piwik_service['base_url'].replace('https','http'), […]

Ça m'ennuie quand même vraiment d'avoir ce code, lié à une particularité du moment, et s'il doit vraiment être mis en place parce que sinon c'est trop compliqué, ça m'ennuie de l'avoir sans la moindre explication d'accompagnement. (et ça doit être un commit distinct).

Concernant "get hobo_json from stdin", le fonctionnement des autres hobo_notify c'est d'utiliser stdin quand le chemin passé vaut "-"; le même comportement devrait être adopté ici.

C'est un vieux patch qui a été attaché à nouveau ? (i.e. il ne répond pas du tout à mon commentaire comme quoi il faut regarder ce qui est passé et si c'est "-" alors utiliser stdin).

Aussi, il mêle un "elif cmd == 'hobo_notify'" qui n'a aucun rapport avec le sujet du commit.

0004-piwik-manage-get-hobo_json-from-stdin-10226.patch : le code faisait avant sys.argv², fait maintenant sys.argv³, comment ?

0001-piwik-manage-use-rotating-file-logger-10226.patch : même si le code ne change pas vraiment ça, [%(name)s.%(module)s.%(funcName)s] ça va donner [__main__.piwik-manage.<module>], c'est pas terrible.

0002-piwik-manage-use-http-when-https-connection-fails-10.patch : je continue à trouver ça très très bizarre et pas vraiment désirable, certainement pas sans une bonne explication du pourquoi, dans le code.

Pour le 0001 la pratique c'est d'envoyer à syslog, voir graylog, pas dans un fichier.

Frédéric Péters a écrit :

0004-piwik-manage-get-hobo_json-from-stdin-10226.patch : le code faisait avant sys.argv², fait maintenant sys.argv³, comment ?

Oui, pour la simple raison qu'au départ, j'avais omis le base_url (J'aurai du faire un autre patch)

0001-piwik-manage-use-rotating-file-logger-10226.patch : même si le code ne change pas vraiment ça, [%(name)s.%(module)s.%(funcName)s] ça va donner [__main__.piwik-manage.<module>], c'est pas terrible.

J'avoue,

0002-piwik-manage-use-http-when-https-connection-fails-10.patch : je continue à trouver ça très très bizarre et pas vraiment désirable, certainement pas sans une bonne explication du pourquoi, dans le code.

• travailler uniquement en http
• faire écouter piwik.monitoring.entrouvert.org sur 443

Benjamin Dauvergne a écrit :

Pour le 0001 la pratique c'est d'envoyer à syslog, voir graylog, pas dans un fichier.

Je corrige

Vu que la machine piwik est hébergée sur didier, c'est didier qui fait proxy. Le script piwik-manage.py lui s'execute sur la vm piwik. Maintenant que tu me fais reflechir
à ça, j'ai 2 solutions pour l'instant :

• travailler uniquement en http
• faire écouter piwik.monitoring.entrouvert.org sur 443

Thomas a fait la config pour que Piwik écoute sur le 443. Je vais modifier le code en conséquence

Faudrait virer les patchs 1 et 2, globalement il faudrait rebaser (il y a des scories dans le patch 4 aussi); aussi il y a une typo dans sudo-hobo-agent à la fin,

1. Enlever le .py à piwik-manage
2. Ne pas logger le ValueError, encadrer plutôt tout le traitement dans un try/except: log.exception() histoire de pouvoir brancher sentry, graylog, etc.. à un moment
4. Ajouter ce qu'il faut pour sentry à partir de la variable d'environnement SENTRY_DSN, voir debian/debian_config_common.py
5. À rebaser dans 0001

Les erreurs dans cmd.deploy() ne sont pas journalisées comme je le faisais remarquer dans mon point 2 plus haut, comme je le disais je mettrai un try/catch autour de tout le bloc « main ».

Dans le patch 5 il me semble que tu as mélangé un changement dans les retours des requêtes d'API (.json()) avec l'ajout du try/except..

Aussi dans tu pourrais mettre quelque chose dans le log.exception('').

Ok je pensais plus à un truc du genre exception(u"Error in piwik-manage hobo-deploy: %s", e).

Je me demande si le sys.exit() ne va pas se prendre les pieds dans le except(Exception) (c'est équivalent à raise SystemExit(x) normalement).

Benjamin Dauvergne a écrit :

Je me demande si le sys.exit() ne va pas se prendre les pieds dans le except(Exception) (c'est équivalent à raise SystemExit(x) normalement).

Après test, non il n'est pas capturé

Permet moi d'en douter:

$ python
>>> import sys
>>> try:
...    sys.exit(1)
... except:
...    print 'exit'
... 
exit

Non tu as raison, SystemExit n'a pas Exception pour parent, ack.