-
Notifications
You must be signed in to change notification settings - Fork 255
Adding API Access to Plugin Context (Updated) #57
Conversation
Update Naming conventions and Docs
small bug with global extension
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 |
flake8 wasn't checking this file. ran flake8 against it to make sure it was compliant with coding guidelines
1 similar comment
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:
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. |
@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? |
Sure! I'm thinking it'll work something like this:
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? |
@jammons sounds neat. I can dig it |
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 |
thanks for merging! |
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:
RtmBot.connect()
method that instantiates the slack_client instance has been moved to the__init__
method.import os
and addsos.getcwd()
tosys.path
in order for plugins to be able to import newly added file, client.pyslack_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