feat: add support for say_stream utility#1462
Conversation
WilliamBergamin
added
enhancement
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1462 +/- ##
==========================================
+ Coverage 91.35% 91.43% +0.08%
==========================================
Files 229 232 +3
Lines 7262 7330 +68
==========================================
+ Hits 6634 6702 +68
Misses 628 628 ☔ View full report in Codecov by Sentry. |
mwbrooks
left a comment
mwbrooks
left a comment
There was a problem hiding this comment.
@WilliamBergamin Thanks a bunch for this PR! It's super exciting.
❓ Can you please provide some code for us to manually test? I'm having trouble importing SayStream and I'm unsure if I'm importing the incorrect path or have the sample app configured incorrectly. What is the import path?
zimeg
left a comment
zimeg
left a comment
There was a problem hiding this comment.
🎙️ Leaving a few comments too but an example listener in description might be helpful for later CHANGELOG I agree!
| # TODO: in the future we might want to introduce a "proper" extract_ts utility | ||
| thread_ts = req.context.thread_ts or event.get("ts") | ||
| if req.context.channel_id and thread_ts: | ||
| req.context["say_stream"] = SayStream( | ||
| client=req.context.client, | ||
| channel_id=req.context.channel_id, | ||
| thread_ts=thread_ts, | ||
| team_id=req.context.team_id, | ||
| user_id=req.context.user_id, | ||
| ) |
There was a problem hiding this comment.
👾 thought: Defaulting to different threading behavior for say and say_stream concerns me somewhat.
🔮 ramble: I understand thread_ts is required to stream chat at this time, but if "parent" messages can be streamed in the future we might want to revisit also say behavior?
| assert say_stream.team_id == context.team_id | ||
| assert say_stream.user_id == context.user_id |
There was a problem hiding this comment.
🧪 suggestion: We might want to also assert that team_id and user_id are defined too if this isn't clear elsewhere!
| called = {"value": False} | ||
|
|
||
| @app.message("") | ||
| def handle_user_message(say_stream: SayStream): |
There was a problem hiding this comment.
| def handle_user_message(say_stream: SayStream): | |
| def handle_bot_message(say_stream: SayStream): |
🔍 suggestion: To match the event!
| @assistant.thread_started | ||
| def start_thread(say_stream: SayStream): | ||
| assert say_stream is not None | ||
| assert isinstance(say_stream, SayStream) | ||
| assert say_stream.channel_id == "D111" | ||
| assert say_stream.thread_ts == "1726133698.626339" | ||
| called["value"] = True |
There was a problem hiding this comment.
🧪 question: Can we also assert that user_id and team_id are set here? Or perhaps not? It's unclear to me what we might expect for events from a direct message...
| assert say_stream is None | ||
| assert context.say_stream is None |
There was a problem hiding this comment.
🌟 praise: This is a pleasant assertion to have for application code guards too I think!
| team_id: Optional[str] = None, | ||
| user_id: Optional[str] = None, |
There was a problem hiding this comment.
| team_id: Optional[str] = None, | |
| user_id: Optional[str] = None, | |
| recipient_team_id: Optional[str] = None, | |
| recipient_user_id: Optional[str] = None, |
📚 question: Are these options meant to match bolt conventions more than the API arguments?
⚡ thought: Indirection in these concepts might be confusing and I'd favor matching the API in case new arguments are added but am asking these questions in search of preference most!
📰 ramble: Also I didn't notice this difference until test files FWIW-
3 participants
Summary
This PR aims to introduce a new kwarg
say_stream, it allows developers the ability to easily use aWebClient.chat_streamobject initialized with logical default valuesTesting
scripts/build_pypi_package.shsay_streamCategory
slack_bolt.Appand/or its core componentsslack_bolt.async_app.AsyncAppand/or its core componentsslack_bolt.adapter/docsRequirements
Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.
./scripts/install_all_and_run_tests.shafter making the changes.