Skip to content

Fix #932 RTMClient v2: no aiohttp dependency, more reliable implementation#933

Merged
seratch merged 5 commits intoslackapi:mainfrom
seratch:rtm-v2
Feb 3, 2021
Merged

Fix #932 RTMClient v2: no aiohttp dependency, more reliable implementation#933
seratch merged 5 commits intoslackapi:mainfrom
seratch:rtm-v2

Conversation

@seratch
Copy link
Contributor

@seratch seratch commented Jan 29, 2021 •
edited
Loading

Summary

This pull request fixes #932 - see the issue for details and context.

TODOs:

  • write unit tests
  • add the section in the document (in another PR to control the timing to merge it)

Category (place an x in each of the [ ])

  • slack_sdk.web.WebClient (sync/async) (Web API client)
  • slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
  • slack_sdk.models (UI component builders)
  • slack_sdk.oauth (OAuth Flow Utilities)
  • slack_sdk.socket_mode (Socket Mode client)
  • slack_sdk.rtm.RTMClient (RTM client)
  • slack_sdk.signature (Request Signature Verifier)
  • /docs-src (Documents, have you run ./docs.sh?)
  • /docs-src-v2 (Documents, have you run ./docs-v2.sh?)
  • /tutorial (PythOnBoardingBot tutorial)
  • tests/integration_tests (Automated tests for this library)

Requirements (place an x in each [ ])

  • 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 python setup.py validate after making the changes.

@seratch seratch added enhancement M-T: A feature request for new functionality semver:minor rtm-client Version: 3x labels Jan 29, 2021
@seratch seratch added this to the 3.3.0 milestone Jan 29, 2021
@seratch seratch self-assigned this Jan 29, 2021
@codecov
Copy link

codecov bot commented Jan 29, 2021 •
edited
Loading

Codecov Report

Merging #933 (23772a8) into main (ebd4c78) will decrease coverage by 1.29%.
The diff coverage is 54.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #933      +/-   ##
==========================================
- Coverage   86.05%   84.75%   -1.30%     
==========================================
  Files          57       58       +1     
  Lines        5018     5235     +217     
==========================================
+ Hits         4318     4437     +119     
- Misses        700      798      +98
Impacted Files Coverage Δ
slack_sdk/rtm/v2/__init__.py 54.16% <54.16%> (ø)
slack_sdk/socket_mode/builtin/connection.py 71.04% <100.00%> (+0.13%) ⬆️
slack_sdk/socket_mode/builtin/internals.py 82.14% <0.00%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebd4c78...23772a8. Read the comment docs.

@seratch
Copy link
Contributor Author

seratch commented Jan 29, 2021

You can run the example by:

export SLACK_SDK_TEST_CLASSIC_APP_BOT_TOKEN=xoxb-{your own classic app bot token}

python -m venv .venv
source .venv/bin/activate
python integration_tests/samples/rtm_v2/rtm_v2_app.py

@seratch seratch marked this pull request as ready for review January 29, 2021 05:53
@seratch seratch mentioned this pull request Feb 1, 2021
Closed
3 tasks
stevengill
stevengill approved these changes Feb 3, 2021
View reviewed changes
Copy link
Member

@stevengill stevengill left a comment

Choose a reason for hiding this comment

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

Very impressive to reuse the websocket work from SocketMode for this new RTM library.

@seratch seratch merged commit e1272eb into slackapi:main Feb 3, 2021
@seratch seratch mentioned this pull request Feb 4, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stevengill stevengill stevengill approved these changes

@mwbrooks mwbrooks Awaiting requested review from mwbrooks

Assignees

@seratch seratch

Labels

enhancement M-T: A feature request for new functionality rtm-client semver:minor Version: 3x

Projects

None yet

Milestone

3.3.0

Development

Successfully merging this pull request may close these issues.

RTMClient v2: no aiohttp dependency, more reliable implementation

2 participants

@seratch @stevengill