Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return callback from RTMClient.run_on #490

Merged
merged 3 commits into from Aug 29, 2019
Merged

Return callback from RTMClient.run_on #490

merged 3 commits into from Aug 29, 2019

Conversation

clavin
Copy link
Contributor

@clavin clavin commented Jul 27, 2019

Summary

Returns the callback passed into RTMClient.run_on so that it can be called elsewhere. For example:

@RTMClient.run_on("message")
def magic(**payload):
  print('馃敭')

magic()
# before PR -> errors because `magic` is `None`
# after PR  -> prints 馃敭

Fixes #485.

Requirements

@codecov
Copy link

codecov bot commented Jul 27, 2019

Codecov Report

Merging #490 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #490      +/-   ##
=========================================
+ Coverage   69.58%   69.6%   +0.01%     
=========================================
  Files          15      15              
  Lines        1654    1655       +1     
  Branches       91      91              
=========================================
+ Hits         1151    1152       +1     
  Misses        481     481              
  Partials       22      22
Impacted Files Coverage 螖
slack/rtm/client.py 84.23% <100%> (+0.08%) 猬嗭笍

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 add2757...55f13dd. Read the comment docs.

def fn_used_elsewhere(**payload):
pass

self.assertTrue(fn_used_elsewhere != None)
Copy link

Choose a reason for hiding this comment

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

self.assertIsNotNone(fn_used_elsewhere) would be better.

pass

self.assertTrue(fn_used_elsewhere != None)
self.assertTrue(fn_used_elsewhere.__name__ == "fn_used_elsewhere")
Copy link

Choose a reason for hiding this comment

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

self.assertEqual(fn_used_elsewhere.__name__, "fn_used_elsewhere") would be better.

@@ -17,6 +17,14 @@ def setUp(self):
def tearDown(self):
slack.RTMClient._callbacks = collections.defaultdict(list)

def test_run_on_returns_callback(self):
@slack.RTMClient.run_on(event="message")
def fn_used_elsewhere(**_unusedPayload):
Copy link

Choose a reason for hiding this comment

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

_unused_payload would be better, as it is written with correct casing according to PEP8.

Names are important 馃槃
@tgerdes
Copy link

tgerdes commented Aug 13, 2019

This seems like a useful change also for chaining multiple similar events to the same callback:

@RTMClient.run_on(event="group_deleted")
@RTMClient.run_on(event="channel_deleted")
def channel_deleted(**payload):
    pass

Will currently attach channel_deleted to RTMClient._callbacks["channel_deleted"] but then raise

SlackClientError: The specified callback 'None' is not callable.

But with this patchset would hook up the method to both events.

@RodneyU215 RodneyU215 added Priority: High bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 2x labels Aug 29, 2019
@RodneyU215
Copy link
Contributor

Thanks a lot for this work @clavin as well as for the review @onlined & @tgerdes!

@RodneyU215 RodneyU215 merged commit 99113f0 into slackapi:master Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 2x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Decorated function by RTMClient.run_on cannot be called
4 participants