FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: honor passing bolt logger in default web client installation by WilliamBergamin · Pull Request #1272 · slackapi/bolt-python · GitHub

/ bolt-python Public

fix: honor passing bolt logger in default web client installation - #1272

Merged
WilliamBergamin merged 4 commits into
mainfrom
honnor-passing-bolt-logger-in-default-web-client-installation
Mar 18, 2025
ConversationCommits (4)ChecksFiles changed
Merged

fix: honor passing bolt logger in default web client installation#1272
WilliamBergamin merged 4 commits into
mainfrom
honnor-passing-bolt-logger-in-default-web-client-installation

Conversation

WilliamBergamin commented Mar 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

This aims to resolve #1255

#246 defined passing the Bolt logger as the default logger for WebClient instantiation. Since #714 Bolt creates a new WebClient for each request handling execution, this is to prevent using the singleton pattern.

These changes aim to honor both previous decisions by passing the app client logger into each WebClient created for request handling.

Testing

Default WebClient logger is the Bolt App logger

import os
import logging

from slack_bolt import Ack, App
from slack_bolt.adapter.socket_mode import SocketModeHandler
from slack_sdk import WebClient


logging.basicConfig(level=logging.DEBUG)

app = App(token=os.environ.get("SLACK_BOT_TOKEN"))


@app.command("/sample-command")
def sample_command(ack: Ack, client: WebClient):
    ack()
    assert client.logger.name == app.logger.name


# Start Bolt app
if __name__ == "__main__":
    SocketModeHandler(app, os.environ.get("SLACK_APP_TOKEN")).start()

Custom logger can be set on WebClient

import os
import logging

from slack_bolt import Ack, App
from slack_bolt.adapter.socket_mode import SocketModeHandler
from slack_sdk import WebClient


logging.basicConfig(level=logging.DEBUG)
my_logger = logging.Logger("my_logger")
app = App(client=WebClient(token=os.environ.get("SLACK_BOT_TOKEN"), logger=my_logger))

@app.command("/sample-command")
def sample_command(ack: Ack, client: WebClient):
    ack()
    assert client.logger.name == my_logger.name

if __name__ == "__main__":
    SocketModeHandler(app, os.environ.get("SLACK_APP_TOKEN")).start()

Category

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter
  • Document pages under /docs
  • Others

Requirements

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run ./scripts/install_all_and_run_tests.sh after making the changes.

WilliamBergamin added this to the 1.22.1 milestone Mar 17, 2025
WilliamBergamin self-assigned this Mar 17, 2025

codecov Bot commented Mar 17, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.96%. Comparing base (4c237fe) to head (4173845).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1272   +/-   ##
=======================================
  Coverage   90.96%   90.96%           
=======================================
  Files         222      222           
  Lines        7501     7501           
=======================================
  Hits         6823     6823           
  Misses        678      678

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

WilliamBergamin changed the title Honnor passing bolt logger in default web client installation fix: honor passing bolt logger in default web client installation Mar 17, 2025
zimeg reviewed Mar 17, 2025
View reviewed changes

zimeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

🤔 QQ: Do we want to change the slack_sdk logger if no logger is provided to slack_bolt?

Before these changes I found API requests logged as so:

DEBUG:slack_sdk.web.base_client:Sending a request - url: https://slack.com/api/views.publish

But with the default setup this is changed to:

DEBUG:slack_bolt.App:Sending a request - url: https://slack.com/api/views.publish

I think both cases make sense, but I might be used to having the default slack_sdk logs shown here 📣

Comment thread tests/scenario_tests/test_web_client_customization.py
assert response.body == ""
assert_auth_test_count(self, 1)

def test_custom_web_client_logger_is_used_instead_of_bolt_app_logger(self):

zimeg Mar 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

👏 Awesome checks!

zimeg approved these changes Mar 17, 2025
View reviewed changes

zimeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

A bit more testing is showing that a custom logger setup with the following outputs as expected 🚀

bolt = logging.getLogger("zaps")
logs.setLevel(logging.DEBUG)
app = App(logger=logs, token=os.environ.get("SLACK_BOT_TOKEN"))
DEBUG:zaps:Sending a request - url: https://slack.com/api/views.publish

Even fun combinations of logger setups are logging to the right outputs:

bolt = logging.getLogger("zaps")
bolt.setLevel(logging.DEBUG)
webapi = logging.getLogger("zips")
webapi.setLevel(logging.DEBUG)
app = App(logger=bolt, client=WebClient(token=os.environ.get("SLACK_BOT_TOKEN"), logger=webapi))

LGTM if the comment beforehand seems right to you, but I'm open to more discussion whenever!

zimeg commented Mar 17, 2025

Copy link
Copy Markdown
Member

🏷️ Small nit to the milestone used, we could add this PR to 1.23.0 and include the changes that landed for 1.22.1 with this next release?

WilliamBergamin commented Mar 17, 2025

Copy link
Copy Markdown
Contributor Author

🤔 QQ: Do we want to change the slack_sdk logger if no logger is provided to slack_bolt?

This is a good point to bring up, from my investigation before #714 the default slack_bolt logger was being injected as the slack_sdk logger, the default logger of the parent client in the app is the bolt framework logger.

bolt-python/slack_bolt/app/app.py

Line 242 in 4c237fe

logger=self._framework_logger,

WilliamBergamin merged commit fc58fbf into main Mar 18, 2025
WilliamBergamin deleted the honnor-passing-bolt-logger-in-default-web-client-installation branch March 18, 2025 16:18
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

zimeg zimeg approved these changes
mwbrooks Awaiting requested review from mwbrooks
misscoded Awaiting requested review from misscoded

Assignees

WilliamBergamin

Labels

area:async area:sync semver:minor

Projects

None yet

Milestone

1.23.0

Development

Successfully merging this pull request may close these issues.

The request-scoped WebClient doesn't use the provided logger

2 participants

Back | FazBrowse Home | New Git URL