-
Notifications
You must be signed in to change notification settings - Fork 104
fix: endpoint class fix #551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite as Test (Integration)
participant SettingsOptions
participant Endpoint
participant DeepgramClient
participant WebSocketClient
TestSuite->>SettingsOptions: Create with Endpoint(headers=dict)
SettingsOptions->>Endpoint: Pass headers as dict
TestSuite->>DeepgramClient: Instantiate with SettingsOptions
DeepgramClient->>WebSocketClient: Connect using headers from Endpoint
WebSocketClient-->>TestSuite: Headers received as dict
sequenceDiagram
participant User
participant Endpoint
participant Function
User->>Endpoint: Initialize(headers={'key': 'value'})
Endpoint-->>User: headers as dict
User->>Function: Initialize(headers={'foo': 'bar'})
Function-->>User: headers as dict
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings
deepgram/clients/agent/v1/websocket/options.py (10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (10)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@coderabbitai it seems there is an unused Header class then... |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@lukeocodes, you're right to question this! Let me check the current usage of the Header class in the codebase. 🏁 Script executed:
Length of output: 12023 🏁 Script executed:
Length of output: 1423 You're absolutely right! Based on my analysis, the Current Status of Header ClassStill exported publicly:
But no longer used internally:
RecommendationThe
If you want to remove it, you'd need to:
Would you like me to help you plan the removal of the 🧠 Learnings used
|
I'll take a look at what code rabbit is suggestion now. |
Here's what I found @lukeocodes on Code Rabbit's Feedback Assessment I feel we could go with the 2nd option, full removal given it was broken in the first place. but want to check this approach. cc @naomi-lgbt for thoughts We have 2 options: ✅ Code Rabbit is 100% correct:
Recommendation 1: Phased Removal
Recommendation 2 : Complete Removal Arguments for Removal:
Risk Assessment:
|
Proposed changes
🐛 Problem
The Voice Agent API SDK's
endpoint.headers
structure didn't match the API specification, causing validation errors when configuring external TTS providers like AWS Polly.Expected API Format (per specification):
Previous SDK Implementation:
This mismatch caused AWS Polly configurations to fail with "Error parsing client message" errors.
🔧 Solution
Code Changes
Modified
deepgram/clients/agent/v1/websocket/options.py
:Updated
Endpoint
class:headers
field fromOptional[List[Header]]
toOptional[Dict[str, str]]
__getitem__
method to handle dictionary formatUpdated
Function
class:headers
field fromOptional[List[Header]]
toOptional[Dict[str, str]]
__getitem__
method to handle dictionary formatMaintained Backward Compatibility:
Header
class available for existing codeExample Usage (Now Working):
🧪 Testing
Unit Tests
Created comprehensive test suite (
tests/unit_test/test_unit_agent_endpoint_headers.py
):Integration Test App
Created comprehensive test application (
temp-test/test_aws_endpoint_headers.py
):Test App Results:
Test Scenarios Validated:
📊 Impact Analysis
✅ What's Fixed
✅ What's Preserved
Header
class remains available✅ Verification
options.py
needed changes🚀 Result
The SDK now correctly handles dictionary-format headers for Voice Agent API endpoints, resolving the AWS Polly configuration issue while maintaining full backward compatibility. The fix is validated with comprehensive testing and ready for production use.
Types of changes
What types of changes does your code introduce to the community Python SDK?
Put an
x
in the boxes that applyChecklist
Put an
x
in 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.Summary by CodeRabbit