fix:Add update_handler function for create_stream in python sdk#1406
fix:Add update_handler function for create_stream in python sdk#1406fanlia wants to merge 1 commit intoiii-hq:mainfrom
Conversation
add missing update handler for create_stream
|
@fanlia is attempting to deploy a commit to the motia Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdded Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sdk/packages/python/iii/src/iii/iii.py (1)
1426-1429: Alignupdate_handlerreturn contract and normalization withset_handleranddelete_handler.
update_handleris typed aslist[Any]whilestream.update()returnsStreamUpdateResult | None, inconsistent withset_handleranddelete_handlerwhich both returnAnyand applymodel_dump()normalization. This deviates from the established handler pattern and violates the consistency requirement across stream operations.Suggested adjustment
- async def update_handler(data: Any) -> list[Any]: + async def update_handler(data: Any) -> Any: input_data = StreamUpdateInput(**data) if isinstance(data, dict) else data - return await stream.update(input_data) + result = await stream.update(input_data) + return result.model_dump() if result else None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/python/iii/src/iii/iii.py` around lines 1426 - 1429, The update_handler currently declares return type list[Any] and directly returns stream.update(input_data), but stream.update returns StreamUpdateResult | None and handlers should follow the same pattern as set_handler/delete_handler by returning Any and normalizing responses via model_dump(); change update_handler to return Any, call StreamUpdateInput for dicts as now, await stream.update(input_data), then if result is not None apply result.model_dump() (or equivalent normalization used by set_handler/delete_handler) and return that, otherwise return None to preserve the contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/packages/python/iii/src/iii/iii.py`:
- Line 1437: The create_stream() docstring is incorrect about update not being
registered; update the docstring for create_stream() to reflect that
register_function is called with {"id": f"stream::update({stream_name})"} and
thus an update handler (update_handler) is registered for the stream;
specifically mention that stream::update(<stream_name>) is registered via
register_function and describe the behavior of update_handler (what it
expects/does) so docs align with the implemented registration.
---
Nitpick comments:
In `@sdk/packages/python/iii/src/iii/iii.py`:
- Around line 1426-1429: The update_handler currently declares return type
list[Any] and directly returns stream.update(input_data), but stream.update
returns StreamUpdateResult | None and handlers should follow the same pattern as
set_handler/delete_handler by returning Any and normalizing responses via
model_dump(); change update_handler to return Any, call StreamUpdateInput for
dicts as now, await stream.update(input_data), then if result is not None apply
result.model_dump() (or equivalent normalization used by
set_handler/delete_handler) and return that, otherwise return None to preserve
the contract.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f7dade86-38ad-4b14-ba81-840b53ba8fc7
📒 Files selected for processing (1)
sdk/packages/python/iii/src/iii/iii.py
| self.register_function( | ||
| {"id": f"stream::list_groups({stream_name})"}, list_groups_handler | ||
| ) | ||
| self.register_function({"id": f"stream::update({stream_name})"}, update_handler) |
There was a problem hiding this comment.
Update create_stream() docstring to match the new behavior.
After registering stream::update(...), the docstring still states that update is not registered. Please update the method docs so SDK behavior and documentation stay aligned.
As per coding guidelines, "Check for related inconsistencies with the docs/".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/packages/python/iii/src/iii/iii.py` at line 1437, The create_stream()
docstring is incorrect about update not being registered; update the docstring
for create_stream() to reflect that register_function is called with {"id":
f"stream::update({stream_name})"} and thus an update handler (update_handler) is
registered for the stream; specifically mention that
stream::update(<stream_name>) is registered via register_function and describe
the behavior of update_handler (what it expects/does) so docs align with the
implemented registration.
add missing update handler for create_stream
Summary by CodeRabbit