Fix HTTPX Streaming #448
Fix HTTPX Streaming #448
Conversation
WalkthroughThe recent changes to the Deepgram client files include updates to the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- deepgram/clients/abstract_async_client.py (1 hunks)
- deepgram/clients/abstract_sync_client.py (1 hunks)
Files skipped from review due to trivial changes (1)
- deepgram/clients/abstract_async_client.py
Additional comments not posted (2)
deepgram/clients/abstract_sync_client.py (2)
340-340: Potential resource management issue.The removal of the
withstatement means thehttpx.Clientinstance will not be automatically closed, potentially leading to resource leaks if not properly managed elsewhere.Ensure that the
httpx.Clientinstance is explicitly closed after use to prevent resource leaks.
341-342: Verify the correctness oftransportkey removal.The
transportkey is removed fromkwargsafter thehttpx.Clientinstance is created. Ensure that this change does not affect the intended behavior of the function.Verification successful
The removal of the
transportkey fromkwargsis correct.The
transportkey is used to initialize thehttpx.Clientorhttpx.AsyncClientinstance and is not required for the request itself. Removing it fromkwargsensures that only relevant parameters are passed to the request.
abstract_sync_client.py:transport = kwargs.get("transport") client = httpx.Client(timeout=timeout, transport=transport) if transport: kwargs.pop("transport") req = client.build_request(method, _url, headers=_headers, **kwargs)
abstract_async_client.py:transport = kwargs.get("transport") client = httpx.AsyncClient(timeout=timeout, transport=transport) if transport: kwargs.pop("transport") req = client.build_request(method, _url, headers=_headers, **kwargs)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of `transport` key removal from `kwargs` in `_handle_request_raw`. # Test: Search for the function usage. Expect: Only occurrences of the new behavior. rg --type python -A 5 $'_handle_request_raw'Length of output: 74
Script:
#!/bin/bash # Description: Verify the correctness of `transport` key removal from `kwargs` in `_handle_request_raw`. # Test: Search for the function usage. Expect: Only occurrences of the new behavior. rg --glob '*.py' -A 5 '_handle_request_raw'Length of output: 1704
Script:
#!/bin/bash # Description: Extract the full implementation of the `_handle_request_raw` function from both `abstract_sync_client.py` and `abstract_async_client.py`. # Extract the implementation from `abstract_sync_client.py` ast-grep --lang python --pattern $'def _handle_request_raw($_, $_, $_, $_, $_, $$$) {\n $$$\n}' deepgram/clients/abstract_sync_client.py # Extract the implementation from `abstract_async_client.py` ast-grep --lang python --pattern $'async def _handle_request_raw($_, $_, $_, $_, $_, $$$) {\n $$$\n}' deepgram/clients/abstract_async_client.pyLength of output: 1244
Script:
#!/bin/bash # Description: Extract the full implementation of the `_handle_request_raw` function from both `abstract_sync_client.py` and `abstract_async_client.py`. # Extract the function definition from `abstract_sync_client.py` ast-grep --lang python --pattern 'def _handle_request_raw($_, $_, $_, $_, $_, $$$)' deepgram/clients/abstract_sync_client.py # Extract the function definition from `abstract_async_client.py` ast-grep --lang python --pattern 'async def _handle_request_raw($_, $_, $_, $_, $_, $$$)' deepgram/clients/abstract_async_client.pyLength of output: 950
Script:
#!/bin/bash # Description: Extract the full implementation of the `_handle_request_raw` function from both `abstract_sync_client.py` and `abstract_async_client.py`. # Extract the implementation from `abstract_sync_client.py` rg 'def _handle_request_raw' -A 50 deepgram/clients/abstract_sync_client.py # Extract the implementation from `abstract_async_client.py` rg 'async def _handle_request_raw' -A 50 deepgram/clients/abstract_async_client.pyLength of output: 4080
davidvonthenen
force-pushed
the
fix-httpx-streaming
branch
from
d30f1e7 to
f903063
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- deepgram/clients/abstract_async_client.py (1 hunks)
- deepgram/clients/abstract_sync_client.py (1 hunks)
- examples/requirements-examples.txt (1 hunks)
- examples/text-to-speech/rest/raw/hello_world_play/main.py (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- deepgram/clients/abstract_async_client.py
- deepgram/clients/abstract_sync_client.py
Additional context used
Learnings (1)
examples/text-to-speech/rest/raw/hello_world_play/main.py (1)
Learnt from: dvonthenen PR: deepgram/deepgram-python-sdk#426 File: examples/text-to-speech/rest/file/hello_world_new/main.py:6-6 Timestamp: 2024-07-01T18:17:36.248Z Learning: When reviewing example files in the Deepgram Python SDK, unused imports should not be flagged for removal as they may be needed for quick logging enablement.
Additional comments not posted (3)
examples/requirements-examples.txt (2)
8-8: LGTM!The addition of
sounddevicepackage is appropriate for enhanced audio processing capabilities.
9-9: LGTM!The addition of
numpypackage is appropriate for numerical computations.examples/text-to-speech/rest/raw/hello_world_play/main.py (1)
102-103: LGTM!The entry point is correctly defined and calls the main function.
Proposed changes
After a recent rebase, I broke the httpx stream functionality when doing a compare with meld. This feature hasn't gone through a release yet, so good that I caught it.
Types of changes
What types of changes does your code introduce to the community Python SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
NA
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
Dependencies
sounddevice==0.4.7andnumpy==2.0.1for enhanced audio processing capabilities.