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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ratelimited slack client.conversations_list poorman solution #4094

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nzin-alayacare
Copy link

Description

When configuring a Slack Bot, Onyx try to list all public/private slack channels, to let you propose on which slack channel(s) you want to configure Onyx.
The problem is that on big Slack workspace (in my case there are > 4000 public/private slack channels), the client.conversations_list returns very quickly the first ~ 1000 slack channels, but face a rate limiting issue after that.

The proposed solution is not a prodution-ready solution, but it is a "poor man" solution:

  • if the time to get back the list of all channels is taking more than 10s we failed (we returns a 202)
  • BUT we start a background thread, to continue a "cold start" job to collect all channels
  • when we have finished to collect all channels, we will be able to serve the saved-for-later list, next time someone ask for all channels

How Has This Been Tested?

It was tested on our company workspace, which contains > 4000 slack channels. It "works" but the cold start takes ~ 30 minutes to get the list of all channels.

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

  • This PR should be backported (make sure to check that the backport attempt succeeds)
  • [Optional] Override Linear Check

Copy link

vercel bot commented Feb 22, 2025

@nzin-alayacare is attempting to deploy a commit to the Danswer Team on Vercel.

A member of the Team first needs to authorize it.

@nzin-alayacare
Copy link
Author

cf #4095

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant