je ne vais pas faire une PR GitHub (je ne saurai pas tester que je n'ai rien cassé). Mais comme j'aime bien relire du Python, je me permets quelques petites remarques, parce que le code est plutôt de bonne facture.
À beaucoup d'endroits (pas partout) on trouve des logging.debug("Log in %s" % logfile) ; il est préférable d'écrire logging.debug("Log in %s", logfile) car l'interpolation de chaîne ne sera faite que si le message est affiché (le niveau est DEBUG ou moins dans cet exemple).
logging.debug("ici: %i %s", i, cmd[i]) j'imagine que le "ici" est un reliquat.
if type(tunnel) == AutoTunnel: je pense qu'un if isinstance(tunnel, AutoTunnel): est plus propre (même si au final tester les types c'est pas très élégant, et genre une méthode/propriété "is_auto", par exemple, serait plus appropriée je trouve).
if t.status != 'ESTABLISHED' and t.status != 'LISTEN': => if t.status in ('ESTABLISHED', 'LISTEN'):
Il y a beaucoup d'utilisation de eval() qui gagneraient à être remplacées par des getattr() ; outre le caractère sécurité, on évite aussi de reparser du code à chaque fois (eval() est vraiment à éviter ; je doute m'en être servi en presque 20 ans de Python).
Dans la mesure où Python 3.8 est requis, tu peux utiliser à moult endroits les f-strings qui sont vraiment super sympathiques.
Il y a beaucoup de double-recherches dans les dictionnaires, quand une recherche simple sera plus efficace et souvent plus compacte.
On utilise .extend() et une generator expression pour éviter d'avoir N appels à append() tout en étant plus court:
def__repr__(self):reps=[self.header]# Remarque 1: pas besoin de mettre une autre paire de parenthèses car le generator est le seul paramètrereps.extend(str(tunnel)fortunnelinself.tunnels.values())# Remarque 2: version alternative avec map()# reps.extend(map(str, self.tunnels.values()))return"\n".join(reps)
Avec les nouvelles fonctionnalités d'unpacking de Python 3, on arrive à ce résultat qui a un petit goût de programmation fonctionnelle plutôt cool:
# Petite revue de code
Posté par GuieA_7 (site web personnel) . En réponse à la dépêche Moniteur de tunnels SSH Tunnelmon en version 1.1. Évalué à 10.
Bonjour,
je ne vais pas faire une PR GitHub (je ne saurai pas tester que je n'ai rien cassé). Mais comme j'aime bien relire du Python, je me permets quelques petites remarques, parce que le code est plutôt de bonne facture.
logging.debug("Log in %s" % logfile); il est préférable d'écrirelogging.debug("Log in %s", logfile)car l'interpolation de chaîne ne sera faite que si le message est affiché (le niveau est DEBUG ou moins dans cet exemple).logging.debug("ici: %i %s", i, cmd[i])j'imagine que le "ici" est un reliquat.if type(tunnel) == AutoTunnel:je pense qu'unif isinstance(tunnel, AutoTunnel):est plus propre (même si au final tester les types c'est pas très élégant, et genre une méthode/propriété "is_auto", par exemple, serait plus appropriée je trouve).if t.status != 'ESTABLISHED' and t.status != 'LISTEN':=>if t.status in ('ESTABLISHED', 'LISTEN'):eval()qui gagneraient à être remplacées par desgetattr(); outre le caractère sécurité, on évite aussi de reparser du code à chaque fois (eval()est vraiment à éviter ; je doute m'en être servi en presque 20 ans de Python).Il y a beaucoup de double-recherches dans les dictionnaires, quand une recherche simple sera plus efficace et souvent plus compacte.
peut s'écrire simplement
self.forward = self.forwards.get(forward, "unknown").De la même manière le bout de code suivant est intéressant (car plusieurs améliorations sont possibles)
On utilise
.values()vu qu'on n'a pas besoin de la clé:On utilise
.extend()et une generator expression pour éviter d'avoir N appels àappend()tout en étant plus court:Avec les nouvelles fonctionnalités d'unpacking de Python 3, on arrive à ce résultat qui a un petit goût de programmation fonctionnelle plutôt cool:
==> raddr, rport = c.raddr or (None, None)
En espérant avoir été utile.
Bon week-end !