Produits Entr'ouvert » Passerelle

connector = '%s-%s' % (slugify(unicode(self.__class__.__name__)), self.slug)

Ça me semble plus pratique pour la suite d'avoir dans le modèle deux champs; un qui serait l'appname (il y a déjà une méthode get_connector_slug pour avoir ça) et l'autre pour le slug de l'objet.

def _log(self, levelname, message, request=None, **extra):

logger.whatever n'oblige pas l'évaluation immédiate des arguments, on fait par exemple self.logger.debug('new token: %s (timeout %ss)', token, timeout). On pourrait ne pas perdre ça ?

Il ne faut bien sûr pas logguer ce qui sort du log_level.

Je préférerais aussi vraiment qu'on remplace le self.logger = logging.getLogger(...) actuel par un self.logger = NotreObject, qui fasse le boulot sans demander des modifications dans tous les connecteurs.

connector = models.CharField(max_length=128, verbose_name='connector')

cf remarque plus haut. Aussi pour moi ça pourrait être NULL, j'imagine assez bien qu'on veuille logguer des trucs "généraux" (genre au démarrage logguer le numéro de version de passerelle).

ipsource = models.GenericIPAddressField(blank=True, null=True, unpack_ipv4=True,

unpack_ipv4 est à False par défaut côté Django, pourquoi ça nous déplait ?

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

Ça me semble plus pratique pour la suite d'avoir dans le modèle deux champs; un qui serait l'appname (il y a déjà une méthode get_connector_slug pour avoir ça) et l'autre pour le slug de l'objet.

et tant qu'à faire, ajouter un «endpoint» (qui peut être NULL, comme les autres).

Je préférerais aussi vraiment qu'on remplace le self.logger = logging.getLogger(...) actuel par un self.logger = NotreObject, qui fasse le boulot sans demander des modifications dans tous les connecteurs.

+1

ipsource = models.GenericIPAddressField(blank=True, null=True, unpack_ipv4=True,

unpack_ipv4 est à False par défaut côté Django, pourquoi ça nous déplait ?

J'ai pas d'avis, donc je dirais de laisser Django décider. Et pis ::ffff: ça fait moderne.

def init(self, level, classname=None, appname=None, slug=None):

Je virerais le classname, et je passerais plutôt le appname au getLogger().

Plus loin il est utilisé pour faire resource_type/resource_pk mais comme je suggérais, je préférerais comme champs « un qui serait l'appname (il y a déjà une méthode get_connector_slug pour avoir ça) et l'autre pour le slug de l'objet ».

def debug(self, message, request=None, **extra):

Comme noté dans un commentaire précédent, logger.debug() n'a pas cette signature, on fait par exemple self.logger.debug('new token: %s (timeout %ss)', token, timeout).

Bandeau corail, tu paies ta tournée à l'eoday.

À part ça je voyais les logs dans un pavé supplémentaire du connecteur.

Il faudra au minimum un filtrage par date (voir admin Django elle est bien) parce qu'à 10 lignes par page ça va vite devenir galère à exploiter.

Il faudra au minimum un filtrage par date (voir admin Django elle est bien) parce qu'à 10 lignes par page ça va vite devenir galère à exploiter.

Ouaip; cela étant je laisserais à ce ticket un affichage basique, comme je l'écrivais dans un bloc de la page du connecteur, sur l'idée d'un "systemctl status ..." qui affiche juste les dix dernières lignes.

Ensuite et je vais créer un ticket immédiatement pour ça, l'ajout de fonctionnalités de filtres, puis d'autres encore. (filtres : #14671)

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

Il faudra au minimum un filtrage par date (voir admin Django elle est bien) parce qu'à 10 lignes par page ça va vite devenir galère à exploiter.

Ouaip; cela étant je laisserais à ce ticket un affichage basique, comme je l'écrivais dans un bloc de la page du connecteur, sur l'idée d'un "systemctl status ..." qui affiche juste les dix dernières lignes.

Ca me va d'en avoir 50 par écran dans cette première mouture (ça va vite, les logs passerelle)

- </div>
+ </dv>

Sérieux ?

J'écrivais :

Ça me semble plus pratique pour la suite d'avoir dans le modèle deux champs; un qui serait l'appname (il y a déjà une méthode get_connector_slug pour avoir ça) et l'autre pour le slug de l'objet.

Et je n'ai pas vu de commentaire là-dessus et pourtant tu continues avec un champ connector créé sur base de '%s-%s' % (self.appname, self.slug).

attr['message'] = message

Mais les substitutions de chaine qui peuvent se trouver dedans ?

1. Resource Custom DB Loggger

Kamoulox ?

<th>{% trans 'Id' %}</th>

Sert à rien.

<th>{% trans 'Message preview' %}</th>

J'appellerais juste ça "Message".

passerelle/templates/passerelle/manage/log_detail.html

Laissons l'affichage d'un unique message de log à un autre ticket sinon on s'en sortira pas. (je viens de créer #14682)

<div id="logs"> {% if perms.base.view_resourcelog %}

Plutôt ne pas afficher du tout le pavé.

_logs/

(J'ai noté plus haut qu'on pouvait zapper cette partie pour ce ticket, mais pour l'avenir quand même, évitons les underscores dans les URL.)

Fichier 0001-misc-fix-pep8-errors.patch

Je trouve une certaine valeur à pouvoir naviguer dans l'historique, les modifications d'indentation, elles m'ennuient plus qu'autre chose.

Fichier 0002-misc-add-import-of-PermissionDenied.patch

Certes, mais si tu tombes sur des bugs lors du développement d'une fonctionnalité, n'hésite pas à le noter dans de nouveaux tickets, plutôt que mêler ça à une revue de code pour autre chose.

Fichier 0003-add-integrated-log-system-14191.patch

Pourquoi ce passage à un _message qui est un encodage base64 du message ?

    def __unicode__(self):
        return '%s %s %s' % (self.timestamp, self.loglevel, self.appname, self.slug)

Manque un %s.

loglevel = models.CharField(max_length=16, verbose_name='log level')

En fait ça serait pas mal d'avoir ça sous forme d'entier, ça sera plus facile ensuite pour filter genre "tout ce qui est au-dessus de erreur".

        <th>Id</th>

Pourquoi pas de trad ?

        <th>{% trans 'Ip Source' %}</th>

Dans la description du modèle, le verbose_name est "IP Address", ça sera bien d'avoir la même chose des deux côtés, et que "IP" soit en majuscules, bien sûr.

(pas terminé)

C'est principalement pour mieux gérer tout ce qui est binaire et passe par lq log. Peut etre, une autre solution serait d'utiliser un FileField et d'y mettre tout ce que l'on reçoit comme message de la log.

Ça n'a pas de sens quand après au niveau de l'affichage du tape juste {{record.message}}. Les messages de log, c'est du texte, point.

message = models.TextField(max_length=2048, verbose_name='message') alors que très visiblement ça peut déborder, cf l'actuel REQUESTS_RESPONSE_CONTENT_MAX_LENGTH dont la valeur par défaut est 5000.

log.message = message[:model._meta.get_field('message').max_length]

Le patch ne s'applique pas, peut-être parce que trainent aussi dans ce ticket 0002-misc-add-import-of-PermissionDenied.patch et 0001-misc-fix-pep8-errors.patch. Il y a moyen de les dégager vers leurs propres tickets ? (Et si possible de faire sans le deuxième (sérieusement))

Il y avait une petite part pour toi quand même : C'est chiant à modifier plus tard, faisons le tout de suite. "Source IP", pas "IP Source", ça vaut aussi et surtout pour le nom de l'attribut.

Et ça n'a pas bougé :

                ('ipsource', models.GenericIPAddressField(null=True, verbose_name='IP Source', blank=True)),

Ça vaut aussi et surtout pour le nom de l'attribut.

Josué Kouka a écrit :

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

Ça vaut aussi et surtout pour le nom de l'attribut.

+1

(pas fini)

Je pense que tout est ok now.

Pour vérifier, c'est facile, il suffit de chercher dans le patch "ip source" ou "ip source".

Hint: il en reste un.

Il y a encore de quoi discuter mais passons, ack.

Hint: il en reste un.

Et comme un boulet je ne vérifie pas et bim, le patch qui suit ne corrige pas ça.