Produits Entr'ouvert » Authentic 2

Comme domaine de log oidc_authorization_error me semble bien trop spécifique.

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

Comme domaine de log oidc_authorization_error me semble bien trop spécifique.

Est ce que oidc_error convient ? ça me fait penser à aussi enregistrer les erreurs liées à la vu token

src/authentic2_idp_openid/views.py:logger = logging.getLogger('authentic.idp.idp_openid')

Le même domaine doit être utilisé. Et là je me rends compte que tu es dans le même fichier, tu as donc déjà cet objet logger.

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

[...]

Le même domaine doit être utilisé. Et là je me rends compte que tu es dans le même fichier, tu as donc déjà cet objet logger.

Le bon fichier ne serait pas src/authentic2_idp_oidc/views.py ?

J'en sais rien j'ai lu rapidement déjà trop de temps passé. Astuce : regarder ce qui se fait, faire pareil.

Ça ne vas pas, tu ne peux pas faire ça

logger.debug('%s %s %s'...)

ou ça

content = {...}
...
logger.debug(content, ...)

Il faut journaliser des phrases qui explique ce que décrit la ligne de log et l'évènement qui s'y rattache. Donc plutôt:

logger.debug('idp_oidc: authorization request error redirect_uri=%r error=%r error_description=%r etc...', ...)

Tu peux déclarer ton logger en début de fichier ça ne me gêne plus (en tout cas pas sur les fichiers views.py), pas la peine de tester les logs de debug dans les tests.

Je ne pense pas que le extra soit vraiment bien utile, il faut plutôt y mettre des choses qui permette de trier la ligne de log (ex.: redirect_uri ça permet de trier les erreurs venant d'un site précris, mettre error par contre c'est pas forcément utile).

Les erreurs d'autorisation je mettrai ça en warning plutôt qu'en debug, les erreurs sur le token endpoint je serai moins généreux je ne loggerai rien, ils reçoivent l'erreur c'est quand même assez clair, mais bon sinon en warning aussi.

Ce n'est pas pythonique:

    logger.warning('idp_oidc: authorization request error redirect_uri=%r error=%r error_description=%r'
                   % (redirect_uri, error, error_description), extra={'redirect_uri': redirect_uri})

    logger.warning(u'idp_oidc: authorization request error redirect_uri=%r error=%r error_description=%r'
                   , redirect_uri, error, error_description, extra={'redirect_uri': redirect_uri})

Ça doit faire 20 fois que je te le signale mais toutes les méthodes de log gèrent elles même la chaîne de formatage, nul besoin d'utiliser l'opérateur de formatage, aussi il faut passer une chaîne de formatage unicode parce qu'il y a de l'unicode dans les chaînes de formatage des handlers.

Ack.