Adding API Access to Plugin Context (Updated)

[philipyun]

• 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 been mindful about doing atomic commits, adding documentation to my changes, not refactoring too much.
• I've a descriptive title and added any useful information for the reviewer. Where appropriate, I've attached a screenshot and/or screencast (gif preferrably).
• I've written tests to cover the new code and functionality included in this PR.
• I've read, agree to, and signed the Contributor License Agreement (CLA).

PR Summary

Adds the ability to call the API directly within the plugin context. The concept was adapted from PR #20 written by @zbarahal. Few differences differentiate between their implementation and mine. His requires slightly heavier adjustments to the RtmBot object class definition. Mine only requires two changes in the definition. Additionally mine is compatible and ready to merge with the current master

Notable changes:

• the line in the RtmBot.connect() method that instantiates the slack_client instance has been moved to the __init__ method.
• import os and adds os.getcwd() to sys.path in order for plugins to be able to import newly added file, client.py
• instantiate a global variable, slack_client which can be access directly within plugins to make direct Web API calls.

Related Issues

could potentially fix and close issues #29 #48 #49 and #50 because of the ability to format messages with direct API calls.

Test strategy

Created a sample plugin that calls from the API instance, and prints out each user and their user ID. This can be found in docs/example-plugin/directAPIcall.py

[coveralls]

Coverage increased (+0.2%) to 34.416% when pulling 0fe3cb6 on philipyun103:master into f785ed9 on slackhq:master.

[coveralls]

Coverage increased (+0.2%) to 34.416% when pulling 0285f67 on philipyun103:master into f785ed9 on slackhq:master.

[coveralls]

Coverage increased (+0.2%) to 34.416% when pulling 65c1af4 on philipyun103:master into f785ed9 on slackhq:master.

[coveralls]

Coverage increased (+0.2%) to 34.416% when pulling 80dd994 on philipyun103:master into f785ed9 on slackhq:master.

[philipyun]

sorry about all the stupid commits. Everything should be good now. It took a while for me to figure out the flake8 linting but the build tests all pass now, and Direct API access calls in the plugin context works 100%. Cheers

[coveralls]

Coverage increased (+0.2%) to 34.416% when pulling 6a4b3ea on philipyun103:master into f785ed9 on slackhq:master.

[coveralls]

Coverage increased (+0.2%) to 34.416% when pulling 6a4b3ea on philipyun103:master into f785ed9 on slackhq:master.

[jammons]

Thanks for submitting this PR! I have a few concerns and have been working on how to fix them in my head for a bit, but haven't gotten around to coding it yet due to some other commitments.

Rather than use a global for this, I'd prefer to refactor the Plugins to make it so that they can be be initialized with the slack_client variable as a parameter. I feel like this way is a bit too magical (not your fault, the whole architecture at the moment feels to magical to me).

I'd like to get this package into the state where we can submit it to pypi and people can do things like:

rtmbot = RtmBot(config)
plugin = Plugin(config,..., slack_client)
rtmbot.register(plugin)

Does that seem reasonable?

That would make it a lot easier to make the plugin class have some better enforced standards and prevent the need for the current magic for matching methods on plugins to incoming messages as there will be a registered list of Plugins and the RtmBot class can simply loop through them.

[philipyun]

@jammons that seems fair. The only part thats passing me is how files in the plugin directory can access that slack client attribute in that implementation, perhaps you can explain how that data gets shared?

[jammons]

Sure!

I'm thinking it'll work something like this:

• user will pip install rtmbot
• user creates their plugin, inheriting from a Plugin abstract base class
• user adds the path to their Plugin in the config file
• in core.py we look at the path and search for anything with Plugin in the name and try to do something like this:

for plugin_path in config.paths:
                self.bot_plugins.append(import_string(plugin_path)(client))

It's a bit of work to do and some departure from the current way of doing things, but I think that will work and shouldn't be any more complicated than the current way of doing it if it's well documented.

Thoughts @philipyun103?

[philipyun]

@jammons sounds neat. I can dig it

[jammons]

Alright, the refactoring is taking longer than I'd hoped, so I'm merging this for now to help people in the short term.

Thanks @philipyun103

[philipyun]

thanks for merging!