pinterest / slackminion

A python bot framework for slack
MIT License
22 stars 14 forks source link

Increase logger level on to differentiate with slack_sdk default logger level #70

Closed surong-bel closed 1 year ago

surong-bel commented 1 year ago

Background: Slackminion calls slack_sdk. Within slack_sdk, DEBUG logger level contains raw json responses which are not always relevant with the command usage. Our goal is to simplify Slackminion logs and make it easy for debugging. This PR increases all relevant logs' logger level in Slackminion from DEBUG to INFO, to differentiate with slack_sdk DEBUG level raw json logs.

$ tox
GLOB sdist-make: /home/rsu/code/slackminion/setup.py
py3 recreate: /home/rsu/code/slackminion/.tox/py3
py3 installdeps: -r/home/rsu/code/slackminion/test-requirements.txt
py3 inst: /home/rsu/code/slackminion/.tox/dist/slackminion-2.0.3.zip
py3 installed: aiohttp==3.8.3,aiosignal==1.3.1,async-timeout==4.0.2,asynctest==0.13.0,attrs==22.1.0,black==22.8.0,certifi==2022.9.24,charset-normalizer==2.1.1,click==8.1.3,coverage==4.5.2,exceptiongroup==1.0.4,Flask==2.2.2,frozenlist==1.3.3,future==0.18.2,idna==3.4,importlib-metadata==5.0.0,iniconfig==1.1.1,isort==5.10.0,itsdangerous==2.1.2,Jinja2==3.1.2,MarkupSafe==2.1.1,mock==3.0.5,multidict==6.0.2,mypy-extensions==0.4.3,packaging==21.3,pathspec==0.10.2,pkg_resources==0.0.0,platformdirs==2.5.4,pluggy==1.0.0,pyparsing==3.0.9,pytest==7.2.0,pytest-cov==2.6.1,PyYAML==6.0,requests==2.28.1,six==1.16.0,slack-sdk==3.18.3,slackminion @ file:///home/rsu/code/slackminion/.tox/dist/slackminion-2.0.3.zip,tomli==2.0.1,typed-ast==1.5.4,typing_extensions==4.4.0,urllib3==1.26.12,websocket-client==0.54.0,Werkzeug==2.2.2,yarl==1.8.1,zipp==3.10.0
py3 runtests: PYTHONHASHSEED='1819695478'
py3 runtests: commands[0] | pytest
================================================================================================== test session starts ==================================================================================================
platform linux -- Python 3.7.5, pytest-7.2.0, pluggy-1.0.0
rootdir: /home/rsu/code/slackminion, configfile: setup.cfg
plugins: cov-2.6.1
collected 115 items                                                                                                                                                                                                     

slackminion/tests/test_bot.py ...................                                                                                                                                                                 [ 16%]
slackminion/tests/test_dispatcher.py .......................                                                                                                                                                      [ 36%]
slackminion/tests/test_util.py .                                                                                                                                                                                  [ 37%]
slackminion/tests/test_core/test_acl.py ......................                                                                                                                                                    [ 56%]
slackminion/tests/test_core/test_core.py ..............                                                                                                                                                           [ 68%]
slackminion/tests/test_plugin/test_base.py ...........                                                                                                                                                            [ 78%]
slackminion/tests/test_plugin/test_manager.py ..                                                                                                                                                                  [ 80%]
slackminion/tests/test_slack/test_conversation.py .........                                                                                                                                                       [ 87%]
slackminion/tests/test_slack/test_event.py ........                                                                                                                                                               [ 94%]
slackminion/tests/test_slack/test_user.py ...                                                                                                                                                                     [ 97%]
slackminion/tests/test_slack/test_room/test_im.py ...                                                                                                                                                             [100%]

=================================================================================================== warnings summary ====================================================================================================
.tox/py3/lib/python3.7/site-packages/pytest_cov/plugin.py:193
  /home/rsu/code/slackminion/.tox/py3/lib/python3.7/site-packages/pytest_cov/plugin.py:193: PytestDeprecationWarning: The hookimpl CovPlugin.pytest_configure_node uses old-style configuration options (marks or attributes).
  Please use the pytest.hookimpl(optionalhook=True) decorator instead
   to configure the hooks.
   See https://docs.pytest.org/en/latest/deprecations.html#configuring-hook-specs-impls-using-markers
    def pytest_configure_node(self, node):

.tox/py3/lib/python3.7/site-packages/pytest_cov/plugin.py:201
  /home/rsu/code/slackminion/.tox/py3/lib/python3.7/site-packages/pytest_cov/plugin.py:201: PytestDeprecationWarning: The hookimpl CovPlugin.pytest_testnodedown uses old-style configuration options (marks or attributes).
  Please use the pytest.hookimpl(optionalhook=True) decorator instead
   to configure the hooks.
   See https://docs.pytest.org/en/latest/deprecations.html#configuring-hook-specs-impls-using-markers
    def pytest_testnodedown(self, node, error):

slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_init
  /home/rsu/code/slackminion/.tox/py3/lib/python3.7/site-packages/_pytest/fixtures.py:900: PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.
  slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_init is using nose-specific method: `setup(self)`
  To remove this warning, rename it to `setup_method(self)`
  See docs: https://docs.pytest.org/en/stable/deprecations.html#support-for-tests-written-for-nose
    fixture_result = next(generator)

slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_init
  /home/rsu/code/slackminion/.tox/py3/lib/python3.7/site-packages/_pytest/fixtures.py:916: PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.
  slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_init is using nose-specific method: `teardown(self)`
  To remove this warning, rename it to `teardown_method(self)`
  See docs: https://docs.pytest.org/en/stable/deprecations.html#support-for-tests-written-for-nose
    next(it)

slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_name
  /home/rsu/code/slackminion/.tox/py3/lib/python3.7/site-packages/_pytest/fixtures.py:900: PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.
  slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_name is using nose-specific method: `setup(self)`
  To remove this warning, rename it to `setup_method(self)`
  See docs: https://docs.pytest.org/en/stable/deprecations.html#support-for-tests-written-for-nose
    fixture_result = next(generator)

slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_name
  /home/rsu/code/slackminion/.tox/py3/lib/python3.7/site-packages/_pytest/fixtures.py:916: PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.
  slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_name is using nose-specific method: `teardown(self)`
  To remove this warning, rename it to `teardown_method(self)`
  See docs: https://docs.pytest.org/en/stable/deprecations.html#support-for-tests-written-for-nose
    next(it)

slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_im
  /home/rsu/code/slackminion/.tox/py3/lib/python3.7/site-packages/_pytest/fixtures.py:900: PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.
  slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_im is using nose-specific method: `setup(self)`
  To remove this warning, rename it to `setup_method(self)`
  See docs: https://docs.pytest.org/en/stable/deprecations.html#support-for-tests-written-for-nose
    fixture_result = next(generator)

slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_im
  /home/rsu/code/slackminion/.tox/py3/lib/python3.7/site-packages/_pytest/fixtures.py:916: PytestRemovedIn8Warning: Support for nose tests is deprecated and will be removed in a future release.
  slackminion/tests/test_slack/test_room/test_im.py::TestSlackIM::test_im is using nose-specific method: `teardown(self)`
  To remove this warning, rename it to `teardown_method(self)`
  See docs: https://docs.pytest.org/en/stable/deprecations.html#support-for-tests-written-for-nose
    next(it)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

----------- coverage: platform linux, python 3.7.5-final-0 -----------
Name                                    Stmts   Miss  Cover   Missing
---------------------------------------------------------------------
slackminion/__init__.py                     0      0   100%
slackminion/__main__.py                    49     49     0%   1-81
slackminion/bot.py                        242     83    66%   54, 60, 64, 84, 93-114, 147-151, 162-176, 181, 188, 190, 213-220, 238-242, 257-261, 270-272, 305-306, 315-316, 328-337, 342, 364-367, 370, 390, 394-401
slackminion/dispatcher.py                 169     31    82%   51-59, 75, 86-87, 92, 116, 122, 124-133, 143, 151-153, 216, 221-223, 225-226
slackminion/exceptions.py                   0      0   100%
slackminion/plugin/__init__.py             22      0   100%
slackminion/plugin/base.py                 77     23    70%   9, 73, 75-82, 96-105, 126, 140, 143-147, 168-169, 187, 190
slackminion/plugin/manager.py             117     50    57%   10, 38, 54-58, 77-80, 93, 97-98, 101-105, 108-137, 141-142, 149-161
slackminion/plugins/__init__.py             2      0   100%
slackminion/plugins/core/__init__.py        1      0   100%
slackminion/plugins/core/acl.py           113      5    96%   16, 27, 139, 150, 162
slackminion/plugins/core/core.py           79     18    77%   28, 30, 34, 39, 74, 93, 108, 115-142, 147-148
slackminion/plugins/core/user.py           40     40     0%   1-66
slackminion/plugins/state/__init__.py      11      2    82%   15, 18
slackminion/plugins/state/file.py          14      4    71%   14-15, 18-19
slackminion/plugins/test.py                42     21    50%   14-15, 20, 25-28, 32, 37, 42, 46-49, 53-56, 59, 62, 69, 74, 79
slackminion/slack/__init__.py               3      0   100%
slackminion/slack/conversation.py          48      0   100%
slackminion/slack/event.py                 31      4    87%   29-31, 43
slackminion/slack/rtm_client.py             6      3    50%   10-12
slackminion/slack/user.py                  45      3    93%   27, 31, 68
slackminion/utils/__init__.py               0      0   100%
slackminion/utils/async_task.py           181    139    23%   16-25, 28-30, 35-36, 39, 44-51, 54-56, 59-63, 66-68, 82-84, 87-91, 94-103, 106-128, 131, 134-164, 169-181, 184-194, 197-201, 204-210, 213-214, 217-224, 229-233
slackminion/utils/util.py                  66     41    38%   19, 29-36, 40-95, 140
slackminion/webserver.py                   39     20    49%   25-48, 51-56
---------------------------------------------------------------------
TOTAL                                    1397    536    62%

============================================================================================ 115 passed, 8 warnings in 0.85s ============================================================================================
isort inst-nodeps: /home/rsu/code/slackminion/.tox/dist/slackminion-2.0.3.zip
isort installed: aiohttp==3.8.3,aiosignal==1.3.1,async-timeout==4.0.2,attrs==22.1.0,black==22.8.0,certifi==2022.9.24,charset-normalizer==2.1.1,click==8.1.3,coverage==4.5.2,exceptiongroup==1.0.1,Flask==2.2.2,frozenlist==1.3.3,future==0.18.2,idna==3.4,importlib-metadata==5.0.0,iniconfig==1.1.1,isort==5.10.0,itsdangerous==2.1.2,Jinja2==3.1.2,MarkupSafe==2.1.1,mock==3.0.5,multidict==6.0.2,mypy-extensions==0.4.3,packaging==21.3,pathspec==0.10.1,pkg_resources==0.0.0,platformdirs==2.5.3,pluggy==1.0.0,pyparsing==3.0.9,pytest==7.2.0,pytest-cov==2.6.1,PyYAML==6.0,requests==2.28.1,six==1.16.0,slack-sdk==3.18.3,slackminion @ file:///home/rsu/code/slackminion/.tox/dist/slackminion-2.0.3.zip,tomli==2.0.1,typing_extensions==4.4.0,urllib3==1.26.12,websocket-client==0.54.0,Werkzeug==2.2.2,yarl==1.8.1,zipp==3.10.0
isort runtests: PYTHONHASHSEED='1819695478'
isort runtests: commands[0] | isort --check .
Skipped 3 files
black inst-nodeps: /home/rsu/code/slackminion/.tox/dist/slackminion-2.0.3.zip
black installed: aiohttp==3.8.3,aiosignal==1.3.1,async-timeout==4.0.2,attrs==22.1.0,black==22.8.0,certifi==2022.9.24,charset-normalizer==2.1.1,click==8.1.3,coverage==4.5.2,exceptiongroup==1.0.1,Flask==2.2.2,frozenlist==1.3.3,future==0.18.2,idna==3.4,importlib-metadata==5.0.0,iniconfig==1.1.1,isort==5.10.0,itsdangerous==2.1.2,Jinja2==3.1.2,MarkupSafe==2.1.1,mock==3.0.5,multidict==6.0.2,mypy-extensions==0.4.3,packaging==21.3,pathspec==0.10.1,pkg_resources==0.0.0,platformdirs==2.5.3,pluggy==1.0.0,pyparsing==3.0.9,pytest==7.2.0,pytest-cov==2.6.1,PyYAML==6.0,requests==2.28.1,six==1.16.0,slack-sdk==3.18.3,slackminion @ file:///home/rsu/code/slackminion/.tox/dist/slackminion-2.0.3.zip,tomli==2.0.1,typing_extensions==4.4.0,urllib3==1.26.12,websocket-client==0.54.0,Werkzeug==2.2.2,yarl==1.8.1,zipp==3.10.0
black runtests: PYTHONHASHSEED='1819695478'
black runtests: commands[0] | black --check .
All done! ✨ 🍰 ✨
50 files would be left unchanged.
________________________________________________________________________________________________________ summary ________________________________________________________________________________________________________
  py3: commands succeeded
  isort: commands succeeded
  black: commands succeeded
  congratulations :)
surong-bel commented 1 year ago

@heimrych please take a look. Thanks

jogo commented 1 year ago

Why not just configure the log settings via https://docs.python.org/3/howto/logging.html#configuring-logging-for-a-library?

surong-bel commented 1 year ago

@jogo thanks. We'd like to keep the granularity of different level of logs. That is: slack_sdk related will be kept as DEBUG and slackminion related will be kept as INFO.

This is for the usage when we use another tool (SREbot) to import Slackminion as a dependency. And we can simply specify logger level (in SREbot) to get different level of logs. Our use cases will be 1) SREbot in test mode: print slack_sdk and Slackminion logs and 2) SREbot in prod mode: only print Slackminion logs.

Since Slackminion will be a dependent, does it make sense to not configure logger level for Slackminion's dependency slack_sdk ?

Heimrych commented 1 year ago

LGTM! Unfortunately, I don't have permission to approve this as well, can @jogo help with this? If there are any doubts regarding why this is for, just let me know here or via Slack as well.

Heimrych commented 1 year ago

discussed offline, and the are actually debug logs so they should be left as debug and instead control the log levels being recorded via the standard python tooling around this.

I think it's fine leaving as debug, however, we should include the logging of the payload (even as debug) in the dispatcher.push function (since it's where commands are identified apart the rest of regular Slack messages). Logging ALL messages sent to Slack really pollutes the logs, even for debug standards.

surong-bel commented 1 year ago

Abandon this PR per discussion offline.