Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds multipart/form-data upload support: new Changes
Sequence DiagramsequenceDiagram
participant Client as Client\n(ChatClient / CommonClient)
participant Base as BaseClient\n(BaseClient / AsyncBaseClient)
participant FS as File System
participant HTTP as HTTP Layer
participant Server as Server
Client->>Base: _upload_multipart(path, data_type, file_path, form_fields)
Base->>FS: Read file bytes (sync or via asyncio.to_thread)
FS-->>Base: file bytes
Base->>Base: Infer MIME type, build multipart payload (files, data)
Base->>HTTP: POST multipart/form-data to path
HTTP->>Server: deliver request
Server-->>HTTP: response
HTTP-->>Base: response data
Base-->>Client: StreamResponse[T]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@getstream/chat/async_client.py`:
- Around line 31-47: The upload_channel_file method declares file as
Optional[str] but passes it to _upload_multipart which requires a non-optional
str; change the signature of upload_channel_file (the function named
upload_channel_file) to make file: str (drop Optional) and update any callers if
necessary so a file is always provided, then call _upload_multipart with that
non-optional file value (referencing _upload_multipart and the path_params
usage) to ensure type consistency.
- Around line 50-71: The parameter name "type" in upload_channel_image shadows
the built-in and matches the earlier "Same type issue" — rename the parameter to
channel_type (and update any references) so the signature becomes
upload_channel_image(self, channel_type: str, id: str, ...), then pass it into
path_params as {"type": channel_type, "id": id} when calling _upload_multipart;
update any internal references or callers of upload_channel_image to use the new
parameter name.
In `@getstream/chat/client.py`:
- Around line 50-71: The method upload_channel_image uses a parameter named
"type" which conflicts with the Python built-in and causes the same type/name
issue; rename the parameter to a clear identifier (e.g., channel_type) in the
upload_channel_image signature, update all internal references
(path_params={"type": channel_type} and any uses in the method body), and adjust
any callers or tests to pass channel_type instead of type so the path
interpolation "/api/v2/chat/channels/{type}/{id}/image" still receives the
correct value.
- Around line 31-47: The upload_channel_file sync method declares file as
Optional[str] but passes it to _upload_multipart which requires a non-optional
str; change the method signature in upload_channel_file to accept file: str
(remove Optional and default None) so callers must provide a file, update any
related type hints and docs for UploadChannelFileResponse and ensure form_fields
logic remains the same (this mirrors the async version change to enforce a
non-optional file argument).
In `@getstream/common/async_client.py`:
- Around line 26-37: The async upload_file function's signature uses
Optional[str] for file but passes it to _upload_multipart which requires a
non-optional str; update the upload_file declaration to require file: str
(remove Optional) and adjust any callers/tests that passed None, ensuring
upload_file (in getstream.common.async_client) and its use of _upload_multipart
keep consistent types; reference the upload_file method and the
_upload_multipart call so you change the parameter type and any calling code
accordingly (mirror the same fix applied to the sync CommonClient upload_file).
- Around line 40-58: The type annotation for the `file` parameter in async def
upload_image currently uses Optional[str], which is incorrect; change it to
match the accepted upload types used by _upload_multipart (for example
Optional[Union[str, bytes, BinaryIO]] or your project’s UploadFile alias),
update the import(s) for Union/BinaryIO if needed, and ensure the function
signature in upload_image and any call sites align with _upload_multipart’s
expected file type (refer to the upload_image function and the _upload_multipart
usage).
In `@getstream/common/client.py`:
- Around line 26-37: The upload_file signature declares file: Optional[str] but
passes it directly to _upload_multipart which requires a non-optional file_path;
update upload_file to validate and handle None (or make the parameter required).
Either (A) change the type to file: str and remove the default None so callers
must supply a path, or (B) keep Optional[str] and add an explicit check in
upload_file (if file is None: raise ValueError("file must be provided") or
similar) before calling self._upload_multipart("/api/v2/uploads/file",
FileUploadResponse, file, form_fields=...), ensuring the runtime never passes
None to _upload_multipart; adjust the function signature and any callers
accordingly.
- Around line 40-58: The upload_image function's file parameter is incorrectly
typed as Optional[str]; change its annotation to the correct file-like type
(e.g., Optional[IO[bytes]] or Optional[BinaryIO]) to match how _upload_multipart
expects a file object, update any related imports (typing.IO or typing.BinaryIO)
and adjust docstring/comments if present; ensure the signature in upload_image
and any similar functions (e.g., upload_file) use the same file-like type so
callers and type checkers align with _upload_multipart's handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e2c4896-5308-4b03-ac6c-ab5d1d0e9680
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
getstream/base.pygetstream/chat/async_client.pygetstream/chat/client.pygetstream/common/async_client.pygetstream/common/client.pytests/test_chat_channel.pytests/test_chat_misc.py
Comment on lines
+40
to
+58
| async def upload_image( | ||
| self, | ||
| file: Optional[str] = None, | ||
| upload_sizes: Optional[List[ImageSize]] = None, | ||
| user: Optional[OnlyUserID] = None, | ||
| ) -> StreamResponse[ImageUploadResponse]: | ||
| form_fields = [] | ||
| if user is not None: | ||
| form_fields.append(("user", json.dumps(user.to_dict()))) | ||
| if upload_sizes is not None: | ||
| form_fields.append( | ||
| ("upload_sizes", json.dumps([s.to_dict() for s in upload_sizes])) | ||
| ) | ||
| return await self._upload_multipart( | ||
| "/api/v2/uploads/image", | ||
| ImageUploadResponse, | ||
| file, | ||
| form_fields=form_fields, | ||
| ) |
There was a problem hiding this comment.
Same type issue for upload_image.
Proposed fix
`@telemetry.operation_name`("getstream.api.common.upload_image")
async def upload_image(
self,
- file: Optional[str] = None,
+ file: str,
upload_sizes: Optional[List[ImageSize]] = None,
user: Optional[OnlyUserID] = None,
) -> StreamResponse[ImageUploadResponse]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@getstream/common/async_client.py` around lines 40 - 58, The type annotation
for the `file` parameter in async def upload_image currently uses Optional[str],
which is incorrect; change it to match the accepted upload types used by
_upload_multipart (for example Optional[Union[str, bytes, BinaryIO]] or your
project’s UploadFile alias), update the import(s) for Union/BinaryIO if needed,
and ensure the function signature in upload_image and any call sites align with
_upload_multipart’s expected file type (refer to the upload_image function and
the _upload_multipart usage).
Comment on lines
+40
to
+58
| def upload_image( | ||
| self, | ||
| file: Optional[str] = None, | ||
| upload_sizes: Optional[List[ImageSize]] = None, | ||
| user: Optional[OnlyUserID] = None, | ||
| ) -> StreamResponse[ImageUploadResponse]: | ||
| form_fields = [] | ||
| if user is not None: | ||
| form_fields.append(("user", json.dumps(user.to_dict()))) | ||
| if upload_sizes is not None: | ||
| form_fields.append( | ||
| ("upload_sizes", json.dumps([s.to_dict() for s in upload_sizes])) | ||
| ) | ||
| return self._upload_multipart( | ||
| "/api/v2/uploads/image", | ||
| ImageUploadResponse, | ||
| file, | ||
| form_fields=form_fields, | ||
| ) |
There was a problem hiding this comment.
Same type mismatch applies to upload_image.
The file parameter here also has the same Optional[str] type issue as upload_file.
Proposed fix
`@telemetry.operation_name`("getstream.api.common.upload_image")
def upload_image(
self,
- file: Optional[str] = None,
+ file: str,
upload_sizes: Optional[List[ImageSize]] = None,
user: Optional[OnlyUserID] = None,
) -> StreamResponse[ImageUploadResponse]:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def upload_image( | |
| self, | |
| file: Optional[str] = None, | |
| upload_sizes: Optional[List[ImageSize]] = None, | |
| user: Optional[OnlyUserID] = None, | |
| ) -> StreamResponse[ImageUploadResponse]: | |
| form_fields = [] | |
| if user is not None: | |
| form_fields.append(("user", json.dumps(user.to_dict()))) | |
| if upload_sizes is not None: | |
| form_fields.append( | |
| ("upload_sizes", json.dumps([s.to_dict() for s in upload_sizes])) | |
| ) | |
| return self._upload_multipart( | |
| "/api/v2/uploads/image", | |
| ImageUploadResponse, | |
| file, | |
| form_fields=form_fields, | |
| ) | |
| def upload_image( | |
| self, | |
| file: str, | |
| upload_sizes: Optional[List[ImageSize]] = None, | |
| user: Optional[OnlyUserID] = None, | |
| ) -> StreamResponse[ImageUploadResponse]: | |
| form_fields = [] | |
| if user is not None: | |
| form_fields.append(("user", json.dumps(user.to_dict()))) | |
| if upload_sizes is not None: | |
| form_fields.append( | |
| ("upload_sizes", json.dumps([s.to_dict() for s in upload_sizes])) | |
| ) | |
| return self._upload_multipart( | |
| "/api/v2/uploads/image", | |
| ImageUploadResponse, | |
| file, | |
| form_fields=form_fields, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@getstream/common/client.py` around lines 40 - 58, The upload_image function's
file parameter is incorrectly typed as Optional[str]; change its annotation to
the correct file-like type (e.g., Optional[IO[bytes]] or Optional[BinaryIO]) to
match how _upload_multipart expects a file object, update any related imports
(typing.IO or typing.BinaryIO) and adjust docstring/comments if present; ensure
the signature in upload_image and any similar functions (e.g., upload_file) use
the same file-like type so callers and type checkers align with
_upload_multipart's handling.
Address code review feedback: change `file` from `Optional[str]` to `str` in all upload method overrides to prevent runtime errors when None is passed to the multipart upload helper. Made-with: Cursor
Made-with: Cursor # Conflicts: # getstream/base.py # tests/test_chat_misc.py
Format sync and async chat client upload helpers so Ruff format check passes in CI. Made-with: Cursor
mogita
approved these changes
Apr 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Tests