slackapi / slackapi/python-slack-sdk

Use logger.isEnabledFor(logging.DEBUG) instead of logger.level <= logging.DEBUG for debug guards

Ouverte
#1,957 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

auto-triage-skip bug
Langage dominant
Python
Étoiles
4k
Forks
857
Merge moyen
22 h 21 min
PR mergées (30 j)
16

Description

Summary

Throughout Socket Mode, debug logs are guarded like this:

if self.logger.level <= logging.DEBUG:
    self.logger.debug(f"... {expensive_call()} ...")

The guard exists to avoid building the message string when debug is off (the f-string argument is evaluated eagerly, before debug() can no-op it). That's a valid goal — e.g. builtin/client.py calls debug_redacted_message_string(message), and client.py calls self.message_queue.qsize() inside the message.

But logger.level is the wrong check: it's only the level explicitly set on that exact logger, defaulting to NOTSET (0). These loggers are created with logging.getLogger(__name__) and setLevel() is never called on them. So with the usual logging.basicConfig(level=logging.INFO) (which configures the root logger), logger.level stays 0, 0 <= 10 is always True, and the guard passes anyway — the expensive string still gets built. The optimization silently does nothing in the most common setup.

Suggested change

Replace:

if self.logger.level <= logging.DEBUG:

with:

if self.logger.isEnabledFor(logging.DEBUG):

isEnabledFor() uses the effective level (walking up the logger hierarchy via getEffectiveLevel()), so it correctly short-circuits when logging is configured at the root/parent — which is what the guard was meant to do.

Where the guarded message is cheap (e.g. it only interpolates an already-computed value), the guard could simply be dropped instead.

Scope

Spotted in slack_sdk/socket_mode/, but the same logger.level <= logging.DEBUG idiom appears ~88 times across ~23 files in slack_sdk/ (webhook, scim, web, audit_logs, rtm, oauth, …). isEnabledFor is currently used nowhere. Worth deciding whether to fix Socket Mode only or apply the change project-wide.

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Commencez par rechercher logger.level <= logging.DEBUG sous slack_sdk/, puis examinez les exemples de Socket Mode dans slack_sdk/socket_mode/, notamment builtin/client.py et client.py. Déterminez si la modification s’applique uniquement à Socket Mode ou aux quelque 88 occurrences ; le travail est terminé lorsque les vérifications de débogage utilisent les niveaux effectifs du logger et que les occurrences dans l’ensemble du projet sont traitées de manière cohérente, en conservant les guards uniquement lorsqu’elles évitent un travail coûteux.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
python
Domaine
api, backend
Type d'issue
Refactorisation
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Active
Clarté
Plutôt claire
Accessibilité débutants
55/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.