Produits Entr'ouvert » Authentic 2

Paul Marillonnet a écrit :

Frédéric Péters (absent jusqu’au 13/6) a écrit :

(présentement je suis sur un écran qui affiche cette phrase pour les deux annuaires configurés et pour l'un j'ai vu "socket timeout error" via journalctl mais pour l'autre aucune idée).

Je me demande si c’est toujours pas cette même lacune du backend ldap qui lâche l’affaire lorsqu’un des annuaires est en erreur, même s’il y en a d’autres ensuite.
Je regarde.

Et pas du tout, mauvaise langue je suis, c’est dû à un bogue dans mon code d’affichage de cette page.

C’est corrigé ici, avec :
· 0001 modification de l’interface pour retrouver l’éventuel message d’erreur lors de la tentative de connexion au(x) serveur(s) ;
· 0002 la remontée du message d’erreur dans le gabarit pour affichage sur la page d’information technique.

Oui tu as raison, à ma connaissance il n’y a pas d’usage simultané possible des deux, et donc c’est mieux de lever une exception.

Voilà, avec l’argument par mot-clé raises.

Pour tout te dire, je réalise entre l’écriture des deux versions du patche qu’un bloc de configuration LDAP peut prendre plusieurs urls, d’où la tentative (foireuse) d’agréger les possibles erreurs qui peuvent survenir pour chaque URL. Dans les faits, je ne sais pas du tout quel est l’usage réel des multiples urls pour un bloc de config, et à ma connaissance on a ça nulle part dans nos raccordements. Comme tu dis, faisons simple, nouvelle version où je prends la dernière erreur remontée de l’itérateur.

Valentin Deniaud a écrit :

Et il me semble qu'on peut enchaîner sur une nouvelle simplification, faire en sorte que l'itérateur ne lève l'exception qu'une fois toutes les url tentées et aucun objet connexion obtenu (genre mettre le if raises après la boucle for). Ça permet de ne pas avoir à modifier get_connection au delà de l'ajout du paramètre.

Bien vu, ça simplement sensiblement 0001 en effet, merci.

Aussi 0002 un peu retouché, notamment parce que [1] j’avais une erreur d’affichage dû au contexte manquant dans le bloc de traduction ({% blocktrans with … %}), avec la correction du test qui va avec, et [2] toujours dans ce test je mocke la méthode get_connections plutôt que celle qui l’appelle, get_connection.

Valentin Deniaud a écrit :

Nickel, un dernier petit truc, pour l'usage qu'on en fait ça ne peut pas se produire mais ça ne coûte rien : actuellement il me semble que taper un list(cls.get_connections(block, raises=True)) alors qu'il n'y a pas d'erreur va crasher (genre errmsg n'est pas défini). Mettre plutôt if raises and errmsg ?

Oui complètement, un oubli de ma part, merci. Ce ne serait pas pas une NameError (errmsg est déclarée en dehors de la boucle for dans cette méthode get_connections), mais une LDAPError(None) alors que potentiellement l’itération s’est déroulée sans encombre. C’est encore pire :)

commit fbf911389de663abbf6bd82919dca70f526fe5b2
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date:   Thu Jun 30 10:43:11 2022 +0200

    translation update (#65491)

commit 4e7a68736476d3e9edd2e117bd5b14e1509e56d2
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date:   Mon Jun 27 15:52:22 2022 +0200

    ldap: display server error on technical info backoffice page (#65491)

commit 3448753eb5aba5e6858acc4febb81b1099efe295
Author: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date:   Mon Jun 27 15:26:58 2022 +0200

    ldap: provide a 'raises' keyword-argument flag on connection retrieval (#65491)