search

dimagi / open-chat-studio

A web based platform for building Chatbots backed by Large Language Models
BSD 3-Clause "New" or "Revised" License
14 stars 7 forks source link

Slack messaging provider and channels configuration #451

Closed snopoke closed 3 months ago

snopoke commented 3 months ago

Resolves: https://github.com/dimagi/open-chat-studio/issues/446

This allows Slack to be used as a messaging provider.

See README.md for more details.

Peek 2024-06-13 13-27

snopoke commented 3 months ago

Overall it looks great!

One question: Why should the SlackChannel override the _ensure_sessions_exists and basically do session management outside of the channel like web channels do? If we include the slack_user (L58 in slack_listeners.py) in the SlackMessage and implement get_chat_id_from_message on the SlackChannel, then we can follow the "normal flow" by the looks of it. I'm not 100% against the current implementation, but just super wary

The main thing is that we can have multiple active sessions per 'user'. Each slack thread is a session and a user can start new sessions at any time. This is very similar to how 'Web' sessions work. I've updated the readme with more details.

SmittieC commented 3 months ago

Overall it looks great! One question: Why should the SlackChannel override the _ensure_sessions_exists and basically do session management outside of the channel like web channels do? If we include the slack_user (L58 in slack_listeners.py) in the SlackMessage and implement get_chat_id_from_message on the SlackChannel, then we can follow the "normal flow" by the looks of it. I'm not 100% against the current implementation, but just super wary

The main thing is that we can have multiple active sessions per 'user'. Each slack thread is a session and a user can start new sessions at any time. This is very similar to how 'Web' sessions work. I've updated the readme with more details.

Thanks, that makes sense. This then reveals two possible paradigms for channels i.e. single threaded + multi threaded. Well, it was there all along, we just supported single threaded external channels up to this point. Although this distinction is revealed in the code, I feel like it's a bit sloppy atm and I would ideally like to see it more emphasized. I'm not 100% sure how yet, but my gut tells me we should do something like that. Ideally within the ChannelBase class to keep things neat. I'll apply my mind a bit

snopoke commented 3 months ago

Should the backends.py file not be updated as well?

no, I've updated the tests to ignore the slack app

snopoke commented 3 months ago

@SmittieC small change to disable slack messaging provider and channels if slack isn't configured