Support for slack events #1451

AgarFu · 2020-09-07T07:16:10Z

This PR is reusing most of the classes from the SlackRTM modules, there is no documentation nor tests in this PR and the main class can be refactored in order to be able to leverage most methods from SlackRTMBackend class.

Please let me know if this is something that you're interested in incorporate to the project I can add the missing bits, it is now in a "works for me" status.

nzlosh · 2020-09-07T09:46:53Z

@AgarFu Nice work, thanks for taking the initiative on creating this PR.

Slack seems to be in a state of disorder with regard to the Python SDK, Application Authorisation model and API access.

From what I can tell, the current situation is as follows:

Python SDK

Comes in two versions v1 (used by slack backend) and v2 (used by slackRTM backend).
Active development on v1 has been halted in favour of v2.

Slack API

RTM - Real Time Messaging (Websocket (client to server) receive all events from the workspace as well as ability to send events to Slack)
Web API - Post events to Slack.
Event API - Subscribe to specific events (server connects to client). Push model only, must reply via Web API.

Application Authentication

"Classic" App tokens that give bots "all" rights scope (strongly urged by Slack to use fine grain scope). Has full access to the RTM and Web API.
New App tokens give fine grain control to what a bot can do. These tokens can't be used with the RTM API, must use Event API and Web API.

This is situation is causing a lot of confusion for users. What the Slack backend needs to do is have a way for the user to express the type of token they're using ("Classic" or "Fine grain") and then adapt it's behaviour accordingly.

In the case of the Event API, the errbot webserver will probably need to used for the Slack server to connection. From what I understand, the event API events and RTM API events should be handled in the same way by errbot so we could probably mutalise code for these 2 use cases.

@sijis @AgarFu What do you think about attempting to merge RTM/Event/Web API support into a single Slack backend?

nzlosh · 2020-09-07T14:22:23Z

Here is the list of events with the RTM/Event API method indicated as reference. https://api.slack.com/events

AgarFu · 2020-09-07T20:27:34Z

Fully agree on all that you said @nzlosh, I'm actually using errbot webserver to listen to posts from Slack. I can refactor SlackRTMBackend in order to be able to share even more between SlackRTMBackend and SlackEventsBackend. Right now these two backends are closer together than SlackRTMBackend and SlackBackend. That will allow a further refactor to fuse all of them into one fully capable client.

Is there a strong opposition to create a backends.slack module with all shared classes between both backends?

sijis · 2020-09-07T21:35:12Z

I'm open to combining it into the SlackRTM backend. I think it logically makes sense.

AgarFu · 2020-09-07T21:42:04Z

Just refactored SlackRTMBackend extracting SlackBackendBase that is doing all the heavy lifting and implementing the handlers for all events.

My new class now only overrides the __init__ and serve_forever methods, I'm dynamically wrapping all possible events coming from slack to methods in the SlackBackendBase, this is much more clean I think.

There are no unit test for SlackRTMBackend (or at least I couldn't find them) so I don't know if that class is still working @sijis would be very nice if you can verify that my refactor did not break the RTM client.

AgarFu · 2020-09-10T17:21:56Z

@nzlosh @sijis any more comments? do you want me to try to refactor this patch further and fuse SlacmRTM and SlackEVENTS?

sijis · 2020-09-12T03:25:23Z

I will try to look into the details of this PR within the next week. I can provide feedback and more comments then.

…nough_permissions

AgarFu · 2020-11-09T06:24:29Z

Hi it seems that Slack is deprecating more and more APIs, and there is interest in the community to support the slack event api. I can devote some more time if you consider that the PR needs rework.

I've rebased my branch with the latest changes in the main branch and also fixed some errors due to these deprecations.

Also the new slack_sdk python module is close to be ready, I think errbot should migrate to the new sdk sooner rather than later but that may have implications in the RTM support.

@nzlosh @sijis thoughts?

…k_event_api_support

Fixed rendering issue in docs

fix: merging configs via --storage-merge cli

* Added email property for slack backend * Updated TestPerson with email property * Updated core_plugin whoami command with email * Added missing blank line * fix: whoami format output Co-authored-by: Sijis Aviles <[email protected]>

This should provide equal behavior as travisci.

This will allow the package to be uploaded to pypi, as it was failing checks.

This should ensure that existing and external backends do not break with when this property is not defined.

* fix: username to userid method signature * Fix username_to_userid and raise when username not uniquely identified (errbotio#1447) * style: fix codestyle warnings Co-authored-by: Carlos <[email protected]>

…rrbot into slack_event_api_support

AgarFu · 2020-11-26T00:48:47Z

Sure thing let me take a look, thx @nzlosh

…eration

AgarFu · 2020-11-26T01:11:27Z

I'll integrate the changes in a different branch for now, will add some tests and then push the changes to my branch I just want to make sure the latest changes that I applied to fix deprecation errors are still in place. I didn't add the proper tests two weeks ago but this PR is substantially more aggressive and still there are no tests for the backend.

For now the changes are https://github.com/AgarFu/errbot/tree/slack_event_api_refactor

I honestly don't like to have a file with 1.4k lines of code and this many classes in it, I'd like to add a new module inside backends, _slack maybe? and split that massive file in smaller ones, it will be easier to maintain and less discouraging to cover it with tests. @nzlosh thoughts?

cc: @sharpyy @kaosx5s

…ack_event_api_refactor

nzlosh · 2020-11-26T07:22:36Z

@AgarFu I agree with you, but I'd like to see this PR merged as a MVP so we can start iterating on refactoring/unit tests/bug fixes and documentation for this new backend.

A deprecation plan has been proposed here #1480, which I hope shows why multiple slack backends is harming errbot's user experience.

The SlackRTM backend was meant to be the successor, but with slack events and the slack client bump to v3, it lacked the features to cover all use cases.

AgarFu · 2020-11-26T17:52:21Z

Ok let me confirm that my team is using a different branch and I'll merge your changes, I'd like to coordinate with you for the next iterations I'd like to at least add the testing as I said before to make sure we do not have regressions and we also have a path going forward to update to the latest versions of slack sdk

Slack event api refactor

AgarFu · 2020-11-26T17:59:34Z

@nzlosh your changes are merged in my branch feel free to merge them into mainstream, THX!

…rrbot into slack_event_api_support

nzlosh · 2020-12-16T00:10:04Z

@AgarFu would you mind merging the latest changes from my repository https://github.com/nzlosh/errbot/tree/slack_event_api_support into your slack_event_api_support branch? I don't know why, but each time I attempt to create a PR against your repository, github doesn't display it in the list of available base repositories.

@sijis Once the changes have been merged into @AgarFu's repository, the slackv3 backend will be ready to be merged to master.

The slackv3 requirements include the RTM and Events python modules. The backend can automatically detect which token type is being used and will initialise the correct client. Not all events are supported by both API types. However the events that errbot requires which are common to both APIs end up calling the same methods.

AgarFu · 2020-12-16T00:22:55Z

will do

sijis · 2020-12-19T06:43:36Z

This is needing a rebase after the latest release. sorry folks.

nzlosh · 2020-12-19T10:51:00Z

@AgarFu I've merged master and pushed to my branch.

AgarFu added 3 commits September 6, 2020 23:42

Initial support for slack events

9555627

Reusing the slack_rtm.py methods

dcf7306

Removing temporal patches

709d174

AgarFu added 2 commits September 7, 2020 14:33

Refactoring SlackRTM

1a991b0

Merge branch 'no_enough_permissions' into slack_event_api_support

e0fa7d1

byronwolfman mentioned this pull request Oct 3, 2020

Allow Slack backend to optionally use immutable userids for ACLs #1421

Closed

nzlosh mentioned this pull request Oct 16, 2020

I'm Unable to use Errbot with Slack as a Botuser #1417

Closed

5 tasks

nzlosh mentioned this pull request Oct 26, 2020

Add Slack Events backend #1472

Closed

AgarFu added 2 commits November 8, 2020 13:30

Merge branch 'master' of https://github.com/errbotio/errbot into no_e…

5facafe

…nough_permissions

Fixing deprecation errors

277376f

AgarFu and others added 12 commits November 8, 2020 22:25

Merge branch 'master' of https://github.com/errbotio/errbot into slac…

814536e

…k_event_api_support

Merge branch 'no_enough_permissions' into slack_event_api_support

63a5fa7

Refactoring SlackRTM

882e097

fixed rendering issue in docs (errbotio#1452)

930db2a

Fixed rendering issue in docs

fix: merging configs via --storage-merge cli (errbotio#1450)

a373b57

fix: merging configs via --storage-merge cli

fix: Add missing email property to backends (errbotio#1456)

22ddf1c

chore: Add github actions (errbotio#1455)

ee1f27c

This should provide equal behavior as travisci.

chore: update changelog and vcheck for 6.1.5

d7197c8

fix: Add content type to package long description

2d1250f

This will allow the package to be uploaded to pypi, as it was failing checks.

fix: Set email property as non-abstract (errbotio#1461)

c310e22

This should ensure that existing and external backends do not break with when this property is not defined.

fix: username to userid method signature (errbotio#1458)

f55d804

* fix: username to userid method signature * Fix username_to_userid and raise when username not uniquely identified (errbotio#1447) * style: fix codestyle warnings Co-authored-by: Carlos <[email protected]>

nzlosh added 3 commits November 25, 2020 23:45

Merge branch 'slack_event_api_support' of https://github.com/AgarFu/e…

c950d55

…rrbot into slack_event_api_support

Align backend variable names with slacksdk names.

1ffdacc

Clean up established network connection when backend is shutdown.

6e4ebdd

Merge branch 'master' of https://github.com/errbotio/errbot into coop…

23768b4

…eration

Merge remote-tracking branch 'carlos/slack_event_api_support' into sl…

7a33fe8

…ack_event_api_refactor

Merge pull request #2 from AgarFu/slack_event_api_refactor

821ee6a

Slack event api refactor

AgarFu added 13 commits November 26, 2020 16:48

Skipping slack tests

c5dad0d

Merge branch 'slack_event_api_support' of https://github.com/AgarFu/e…

b69f3f5

…rrbot into slack_event_api_support

Fixing codestyle

43cc136

Codestyle

ebe28b9

Refactor started, added test for slack markdown infrastructure

a57050a

Making codestyle happy again

cce5e9c

Making codestyle happy

38ded3f

Installing slack_sdk for tests

3ce8dcf

Testing channel name

c684a2a

Not implemented error

f65c444

Refactoring to optimize the cache and reduce duplications

8670371

Small refactor, done testing SlackPerson

72eacc7

100% coverage for person

53db013

sijis added backend: Slack type: feature labels Dec 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for slack events #1451

Support for slack events #1451

AgarFu commented Sep 7, 2020

nzlosh commented Sep 7, 2020

nzlosh commented Sep 7, 2020

AgarFu commented Sep 7, 2020

sijis commented Sep 7, 2020

AgarFu commented Sep 7, 2020

AgarFu commented Sep 10, 2020

sijis commented Sep 12, 2020

AgarFu commented Nov 9, 2020

AgarFu commented Nov 26, 2020

AgarFu commented Nov 26, 2020 •

edited

Loading

nzlosh commented Nov 26, 2020

AgarFu commented Nov 26, 2020

AgarFu commented Nov 26, 2020

nzlosh commented Dec 16, 2020 •

edited

Loading

AgarFu commented Dec 16, 2020

sijis commented Dec 19, 2020

nzlosh commented Dec 19, 2020

Support for slack events #1451

Are you sure you want to change the base?

Support for slack events #1451

Conversation

AgarFu commented Sep 7, 2020

nzlosh commented Sep 7, 2020

Python SDK

Slack API

Application Authentication

nzlosh commented Sep 7, 2020

AgarFu commented Sep 7, 2020

sijis commented Sep 7, 2020

AgarFu commented Sep 7, 2020

AgarFu commented Sep 10, 2020

sijis commented Sep 12, 2020

AgarFu commented Nov 9, 2020

AgarFu commented Nov 26, 2020

AgarFu commented Nov 26, 2020 • edited Loading

nzlosh commented Nov 26, 2020

AgarFu commented Nov 26, 2020

AgarFu commented Nov 26, 2020

nzlosh commented Dec 16, 2020 • edited Loading

AgarFu commented Dec 16, 2020

sijis commented Dec 19, 2020

nzlosh commented Dec 19, 2020

AgarFu commented Nov 26, 2020 •

edited

Loading

nzlosh commented Dec 16, 2020 •

edited

Loading