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

feat: Slack Socket mode and Handler support #1461

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Asher-hss
Copy link
Collaborator

Description

close #1348
solve socket mode and add handler function

Motivation and Context

close #1348
solve socket mode and add handler function

  • I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds core functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)
  • Example (update in the folder of example)

Implemented Tasks

  • Subtask 1
  • Subtask 2
  • Subtask 3

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

  • I have read the CONTRIBUTION guide. (required)
  • My change requires a change to the documentation.
  • I have updated the tests accordingly. (required for a bug fix or a new feature)
  • I have updated the documentation accordingly.

@Asher-hss Asher-hss self-assigned this Jan 19, 2025
@Wendong-Fan Wendong-Fan changed the title Slack Socket mode and Handler support feat: Slack Socket mode and Handler support Jan 20, 2025
@Wendong-Fan Wendong-Fan added the enhancement New feature or request label Jan 20, 2025
@Wendong-Fan Wendong-Fan added this to the Sprint 21 milestone Jan 20, 2025
Copy link
Member

@Wendong-Fan Wendong-Fan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Asher-hss , left some comments below

@@ -55,24 +54,29 @@ class SlackApp:
"/slack/oauth_redirect".
installation_store (Optional[AsyncInstallationStore]): The installation
store for handling OAuth installations.
socket_mode (bool): A flag to enable socket mode for the Slack app,
"""

@dependencies_required('slack_bolt')
def __init__(
self,
token: Optional[str] = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename token to a more specific naming align with Slack's official naming

Comment on lines 89 to 93
socket_mode (bool): A flag to enable socket mode for the Slack app.
(default is False). If want to use socket mode, set it to True.
And Open the socket mode in the Slack API Applications.
url: https://api.slack.com/apps/
oauth_mode (bool): A flag to enable OAuth mode for the Slack app.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the format for default value could be improved for all the docstring in this file, refer to https://github.com/camel-ai/camel/blob/master/CONTRIBUTING.md#3-document-parameters-in-the-args-section

else:
response = result
thread_ts = None
await say(text=response, token=token, thread_ts=thread_ts)

async def on_message(
Copy link
Member

@Wendong-Fan Wendong-Fan Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we ignore messages from the bot itself to avoid infinite loop in async def on_message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bot message is also a type of normal message. I believe we should not assume that users need to filter out this message. this function have provided sufficient parameters to allow users to decide how to filter it themselves.

Comment on lines +299 to +300
if self.mention_me(context, SlackEventBody(**body)):
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you help me understand why we stop it if the bot is mentioned in the message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention messages are also a type of normal message. Therefore, mention messages will be listened to by the message listener. As a result, mention messages will be passed to both the message handler and the mention message handler. Since I have set up a separate mention message handling function, I filter out mention messages received by the message listener.

The response generated by the Slack Bolt handler.
"""
return await self._handler.handle(request)

async def app_mention(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like mention messages are handled both in on_message() and app_mention(), would it trigger duplicate responses?

Copy link
Collaborator Author

@Asher-hss Asher-hss Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, That’s why I added filter `self.mention_me()' in on_message().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[Feature Request] Slackbot Initialization Optimization
3 participants