[SIP-138] Proposal for Slack file upload V2 integration for Alerts and Reports

[eschutho]

Motivation

We are facing an issue with the recent update in Slack's API. Slack has rolled out upload_files_v2 and has discontinued access to any new workspaces on the previous version. This poses a problem as the older instances of Superset cannot be automatically updated to use this new API since there a few differences:

1. The new API accepts only one channel at a time.

2. The new API uses ids instead of names for channels

3. To fetch the channels/translate from ids to names, the Slack workspace admin must add the scope channels:read to their application.

• We cannot leave everyone on the v1 api because new Slack workspaces will not work with the old api.
• We cannot move everyone to v2 because it will be a breaking change as the admin of the workspace will be required to add a new Slack scope/permission.

Proposed Change

Develop a solution that integrates with Slack's new upload_files_v2 API and ensure backward compatibility with older instances of Superset. For converting channels from names to ids, these are two options:

1. Continue to save channel names and always fetch the ids from the slack api in order to upload files to the new v2, provided the scope is available to use the v2 api, otherwise, use the v1 api. At some point we will need to run a migration that will convert from names to ids, but I am wary of calling a third party api during a migration, as we don’t want to be dependent on either the api’s availability or any rate-limiting that might block the migration and thus a version upgrade.

2. Save the channels as ids as we run the reports, creating a new NotificationMethod for SlackV2 for any report that has the correct scopes in place. In this option, we don’t need to call the api every time (unless the admin hasn’t updated their Slack scopes), and it will set us up later for when the v1 api is eventually dropped, or we can remove it from Superset during a major version bump.

In this option we also have two other choices:

1. Save only the report that is being run. This option is cleaner code-wise because it limits the scope of the responsibility of the report executor, but after a period of time when all reports have run and have been converted, there still may be some reports that haven’t been activated that need to be converted.

2. Save all Slack reports when the first V1 Slack report is executed. This would limit the need for multiple updates, and will convert any inactive reports as well, but it will add more complexity to the report command.

In this POC, I am proposing that when running a Slack (v1) Notification we check to see if the scopes are available to fetch channels. If they are, we will save the ids as channels and update the Notification of the ReportSchedule to SlackV2 .

On the front end UI, I have added an auto complete async selector that calls a new API and fetches all Slack channels. It also can take a search field. Slack’s api is paginated but does not have a search option, so all channels will need to be fetched before the search can be applied.

This UI selector can take either names and convert to ids or ids and convert to names. V1 channels will be names, and V2 channels will be saved as ids, but will need to be converted to names for users to understand which channels their messages are being sent to. The UI will fetch all the channels on load. If the fetch is successful, it means that the necessary scopes are in place for the v2 api, and it will create or update an existing Slack notification to V2 when edited.

slackv2flow.mov

During this time, we will log warnings in the Superset log stream to alert administrators to update their Slack workspaces.

Feature flag

By providing a feature flag, we can reduce the load of Slack api calls being made unnecessarily if a Slack workspace owner hasn’t updated the scopes. Because we want to give Superset admins notifications that the Slack v1 api is deprecated and inform them to update the scope, I would recommend to have this Feature Flag on by default, but give Superset admins the option to turn it off temporarily if it causes any issues with the Slack api, especially if they are getting rate limited.

New or Changed Public Interfaces

The new public interface would include an api /report/slack_channels that can be used to fetch all active slack channels associated with the slack token. It will take a search string, and although the slack api is paginated, I do not feel that this api needs to be paginated, as it only returns the channel id and name, as opposed to the Slack API which returns much more information about the channels. This api will also have the same permission scopes as can_report:post or in other words, the same permissions needed to create a report.

New Dependencies

This proposed change will make us dependent on the new Slack's upload_files_v2 API.

Migration Plan and Compatibility

When we drop support for Slack’s file upload v1 api, during a major Superset version bump, we have a few options:

1. Run a migration that changes all Slack (v1) Notifications to SlackV2 but leave the channels as names. When running the report, if we receive a no_channel_found error, we can attempt to fetch the channels from the Slack api and convert the names to ids. If this succeeds, we can then resave the recipient channels as ids.

2. Additionally (or alternately) once the scopes are updated, a report owner can fix the report as before by either editing the report in the UI, which will send the channel names to the api and return the ids, and upon saving, will save the ids.

3. If we want to drop the lookup when a no_channel_found error is received, we can leave option 2 as the required method to “fix” a broken report.

The suggested approach is to look up the ids and save the channels as ids as a SlackV2 Notification and also create all new reports as SlackV2 if the scopes are available. Communicate to admins to update their scopes during this time when both Slack api versions are supported. At some breaking change, 5.0 or 6.0, run a migration that updates all Slack to SlackV2, remove the Slack Notifications and leave the lookup and auto conversions in the codebase for one more major release. At that time, remove the backend lookup and only leave the frontend lookup.

Rejected Alternatives

Options that would involve a breaking change have been rejected, such as updating all reports to v2 and requiring that all Slack workspaces have the new scope in order to continue to use the Slack reports.

As mentioned, calling Slack’s channels api during a migration does not seem to be a feasible option because we cannot guarantee that the scope will be available at the time that the migration is run. Plus there can be other downtime issues with calling a third-party API during a migration.

Considerations

The new channel api conversations_list is also [rate-limited with Tier2 limits](https://api.slack.com/methods/conversations.list).

[michael-s-molina]

Thanks for the detailed SIP @eschutho. Given the constraints, the proposed solution makes sense.

My only suggestion would be to deprecate the code that should be removed during the next breaking window by adding an annotation or comment.

[eschutho]

Thanks @michael-s-molina! 👍

[michael-s-molina]

Closing the SIP as it was approved.

[rusackas]

@eschutho is this done, or are there a few things lingering? Please feel free to move it on the board if you'd like :)

[eschutho]

Just one more small bug fix left. But I can move it off. It's 99% done.