Conversation
📝 WalkthroughWalkthroughAdds a recursive Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Pull request overview
This PR updates the HTTP client layer to omit unset optional fields (None) from JSON request bodies, preventing accidental backend resets during partial updates, and documents the behavior change in the v2→v3 migration guide.
Changes:
- Add a recursive helper to strip
Nonevalues from JSON payloads before sending requests (sync + async). - Apply the stripping logic to all requests that use
kwargs["json"]. - Document the new “omit unset optionals” serialization behavior in
MIGRATION_v2_to_v3.md.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
getstream/base.py |
Strips None values from JSON bodies in both sync and async request paths. |
MIGRATION_v2_to_v3.md |
Adds migration guidance explaining omitted optional fields vs explicit null. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/base.py`:
- Around line 28-35: _strip_none currently filters dict entries based on the
original value and maps lists without removing None results, so None elements
inside lists (and dict values that become None after recursion) remain
serialized; update _strip_none to recurse first then filter out None results: in
the dict branch call _strip_none(v) into a temporary (e.g., v2) and include the
key only if v2 is not None, and in the list branch build a list of
_strip_none(item) results and exclude any that are None (e.g., [r for r in
(_strip_none(i) for i in obj) if r is not None]) so no None/null values are
emitted in nested structures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9440083-bc67-436d-a49b-c2b67d0cef3a
📒 Files selected for processing (2)
MIGRATION_v2_to_v3.mdgetstream/base.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_decoding.py (1)
341-364: Group the new_strip_nonetests into a dedicated test class.Please group these related tests (e.g.,
class TestStripNone:) to match test organization standards for this repository.As per coding guidelines,
**/test_*.py: Keep tests well organized and use test classes to group similar tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_decoding.py` around lines 341 - 364, Group the standalone `_strip_none` tests into a dedicated test class by creating `class TestStripNone:` and moving the functions `test_strip_none_flat_dict`, `test_strip_none_nested_dict`, `test_strip_none_preserves_empty_collections`, `test_strip_none_preserves_list_elements`, and `test_strip_none_passthrough_scalars` inside it, converting each into instance methods (add a `self` parameter) and adjusting indentation; keep the test names and assertions unchanged so they continue to call `_strip_none`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/test_decoding.py`:
- Around line 341-364: Group the standalone `_strip_none` tests into a dedicated
test class by creating `class TestStripNone:` and moving the functions
`test_strip_none_flat_dict`, `test_strip_none_nested_dict`,
`test_strip_none_preserves_empty_collections`,
`test_strip_none_preserves_list_elements`, and
`test_strip_none_passthrough_scalars` inside it, converting each into instance
methods (add a `self` parameter) and adjusting indentation; keep the test names
and assertions unchanged so they continue to call `_strip_none`.
Summary by CodeRabbit
Documentation
Improvements
Tests