Produits Entr'ouvert » Hobo

Je laisse "draft" dans le nom de la branche mais c'est désormais ok.

C'est basiquement comme prévu, avec peut-être comme points à noter :

• dans le diff le gros truc qui apparait c'est le code de hobo_notify.py pour une grande part déplacé dans provisionning/utils.py, on peut me croire, ou inspecter les choses,
• c'est désactivé par défaut,
• ça s'active, par authentic, via HOBO_HTTP_PROVISIONNING = True
• j'avais parlé dans la description de mule uwsgi etc. mais le code authentic créait déjà un thread pour sortir du cycle requête/réponse, je laisse juste ça ainsi,
• une fois activé, si jamais le provisionning HTTP ne fonctionne pas pour certains services, il y a fallback sur l'AMQP,
• il y a un peu de nouveau code, pour gérer le cas particulier #43192 (provisionning vers hobo lui-même), c'est fait de manière totalement adhoc, il y aurait peut-être à un moment à réfléchir pour intégrer un fichier hobo.json et faire que le code "normal" (settings loaders etc.) soit exploité, mais ça sortait du cadre,
• il y a aussi le code côté wcs, https://jenkins.entrouvert.org/job/wcs-wip/job/wip%252F43245-provisionning-draft/

provisionning.py :

Toujours mon avis personnel, mais je serai plus à l'aise avec des opérations sur les set dans notify_agents et moins modifier data, ça enlève le else dans le try et les allers/retours sur data['audience'] :

            rest_audience = set(data['audience']) & set(services_by_url)
            amqp_audience = set(data['audience']) - set(services_by_url)
            data['audience'] = list(amqp_audience)
...
                        json=dict(data, audience=[audience]))

middleware.py:

Même avis que toi pour le code hobo ad-hoc, c'est moche mais ça sort du cadre du ticket, mais quand même pour ne pas qu'on oublie, serait-il possible de faire une méthode séparée de toute la partie if 'hobo.environement' par exemple self.hobo_specific_setup() avec un commentaire signalant notre volonté de voir sa mort prochaine ?

La fin du traitement devrait se faire dans un thread (tout le for i in range(20)), histoire de ne pas se retrouver avec des provisionning foirés à cause du harakiri ou du timeout nginx (et là avec des erreurs de notification coté authentic qui seront fausses si le provisionning s'est bien terminé avant le harakiri). Les seuls cas où authentic lèvera une erreur c'est sur une perte de connectivité, sur un problème de provisionning l'erreur ne viendra que de la brique et c'est mieux.

Pour parler :

• dans hobo.context_processor : la requête est disponible dans theme_base(), mais surtout hobo devrait avoir THEME_SKELETON_URL dans ses settings et aucune donnée utilisée ici ne dépend de la requête,
• dans hobo.deploy.signals: signal request_finished, de toute façon plus de requête dans StoreRequestMiddleware à ce moment (c'est après le passage des middleware)

Bon c'est pour plus tard mais je pense que ce serait une bonne chose de diminuer les usages de StoreRequestMiddleware dans le futur (certainement en faisant d'hobo un bon citoyen de Publik au niveau settings déjà).

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

Toujours mon avis personnel, mais je serai plus à l'aise avec des opérations sur les set dans notify_agents et moins modifier data, ça enlève le else dans le try et les allers/retours sur data['audience'] :

Je l'avais fait de manière similaire mais au final j'ai préféré le amqp_audience.remove() une fois qu'on sait que la requête est bien passée côté HTTP (pas uniquement sur le côté déclaratif de la présence de provisionning-url) (scénario : mise à jour et corbo pas mis à jour pas redémarré et il se trouve connu comme gérant provisionning-url par Hobo alors qu'en fait, pas).

Ok.

La fin du traitement devrait se faire dans un thread

J'aime assez l'idée que non, qu'une erreur 500 si elle a lieu soit visible dans les logs nginx. La boucle est en fait là sur l'IntegrityError si jamais il y a deux provisionning du même utilisateur en même temps; j'imagine le cas bien rare, mais pour éviter l'harakiri, je pourrais changer cette boucle pour la limiter dans le temps, se donner disons très grand max 5 secondes pour l'appel. (en local les logs nginx m'indiquent 0,042 secondes pour le traitement de provisionning (combo)).

Je ne pensais vraiment pas à des histoires concurrence compliquée (d'ailleurs le range(20) me parait exagéré avec le recul), plutôt l'ajout/retrait d'un rôle par héritage, l'injection d'un CSV ou une synchro LDAP fraîche où on peut avoir des centaines d'utilisateurs provisionnés d'un coup, mais si c'est vraiment 42ms par utilisateur effectivement ça passera dans les 120 secondes en général, jusqu'au jour où on injectera 5000 utilisateurs d'un coup; m'enfin le 504 au bout de 30s sera un peu inutile (on aura pas de 500, sauf bug dans le code de provisionning); 5 secondes c'est insuffisant pour l'ajout d'un rôle à tous les agents d'une grosse métro, et en espérant que uwsgi ne tue par le worker sur une socket fermée par nginx (je ne sais pas comment il se comporte dans ce cas).

commit 0012761656db907e2c037c89ed694f4f76a76651
Author: Frédéric Péters <fpeters@entrouvert.com>
Date:   Mon May 25 21:15:26 2020 +0200

    general: use HTTP API to provision users & groups (#43245)