-
Notifications
You must be signed in to change notification settings - Fork 94
fix:fixes endpoint issue #526
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
Sequence Diagram(s)sequenceDiagram
participant Client
participant Endpoint
Client->>Endpoint: Initialize with headers (dict or list)
Client->>Endpoint: Access headers via __getitem__
Endpoint-->>Client: Return headers as dict or list
Client->>Endpoint: Call to_dict()
Endpoint-->>Client: Return headers as dict
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ Finishing Touches
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
deepgram/clients/agent/v1/websocket/options.py
(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check - static
deepgram/clients/agent/v1/websocket/options.py
[error] 96-96: mypy type checking error: Signature of 'to_dict' incompatible with supertype 'DataClassJsonMixin'. Expected 'to_dict(self, encode_json: Any = ...) -> dict[str, dict[Any, Any] | list[Any] | str | int | float | bool | None]', but found 'to_dict(self) -> dict[Any, Any]'.
🪛 GitHub Actions: Check - lint
deepgram/clients/agent/v1/websocket/options.py
[warning] 96-96: pylint W0221: Number of parameters was 2 in 'DataClassJsonMixin.to_dict' and is now 1 in overriding 'Endpoint.to_dict' method (arguments-differ)
🔇 Additional comments (3)
deepgram/clients/agent/v1/websocket/options.py (3)
5-5
: Appropriate import additionsThe additions of
Union
andDict
to the imports are necessary to support the new type annotation for theheaders
attribute in theEndpoint
class.
80-82
: LGTM: Type annotation change supports both dictionary and list typesThis change allows
headers
to be represented either as a dictionary or as a list ofHeader
objects, which appears to be the core of the bugfix for the endpoint issue.
87-94
: Properly handles both dictionary and list representationsThe
__getitem__
method now correctly handles both cases:
- If
headers
is a list, it ensures each entry is aHeader
object- If
headers
is a dictionary, it preserves the dictionary structureThis implementation properly supports the dual representation of headers.
def to_dict(self) -> dict: | ||
""" | ||
Convert the endpoint to a dictionary, properly handling headers. | ||
""" | ||
result = super().to_dict() | ||
if self.headers: | ||
if isinstance(self.headers, dict): | ||
result["headers"] = self.headers | ||
else: | ||
result["headers"] = {h.key: h.value for h in self.headers} | ||
return result | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Fix the method signature to match parent class
The to_dict
method implementation has a signature that's incompatible with the parent class, which is causing pipeline failures.
- def to_dict(self) -> dict:
+ def to_dict(self, encode_json=False) -> dict:
The error messages from the CI pipeline indicate:
- mypy error: Signature incompatible with supertype 'DataClassJsonMixin'
- pylint warning: Number of parameters was 2 in 'DataClassJsonMixin.to_dict' and is now 1 in overriding method
The method needs to maintain the same signature as the parent class while implementing the additional logic to handle the headers properly.
🏁 Script executed:
#!/bin/bash
# Check the signature of DataClassJsonMixin.to_dict
rg -A 5 "def to_dict" --glob="*.py" | grep -A 5 "class.*DataClassJsonMixin"
Length of output: 77
🏁 Script executed:
#!/bin/bash
# Verify the signature of DataClassJsonMixin.to_dict from the dataclasses-json library
pip install dataclasses-json
python3 - << 'EOF'
from dataclasses_json import DataClassJsonMixin
import inspect
src = inspect.getsource(DataClassJsonMixin.to_dict)
sig = inspect.signature(DataClassJsonMixin.to_dict)
print("Source of DataClassJsonMixin.to_dict:")
print(src)
print("Signature of DataClassJsonMixin.to_dict:", sig)
EOF
Length of output: 1375
Fix to_dict
signature to match DataClassJsonMixin
The parent class defines to_dict(self, encode_json=False)
, so your override must include the same parameter and pass it through to super()
. Without this change, mypy and pylint will continue to report an incompatible signature.
• File: deepgram/clients/agent/v1/websocket/options.py (lines 96–107)
• Update method signature and the call to super().to_dict
- def to_dict(self) -> dict:
+ def to_dict(self, encode_json=False) -> dict:
"""
Convert the endpoint to a dictionary, properly handling headers.
"""
- result = super().to_dict()
+ result = super().to_dict(encode_json=encode_json)
if self.headers:
if isinstance(self.headers, dict):
result["headers"] = self.headers
else:
result["headers"] = {h.key: h.value for h in self.headers}
return result
— Signature verified via inspect.signature(DataClassJsonMixin.to_dict)
returning (self, encode_json=False)
📝 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.
def to_dict(self) -> dict: | |
""" | |
Convert the endpoint to a dictionary, properly handling headers. | |
""" | |
result = super().to_dict() | |
if self.headers: | |
if isinstance(self.headers, dict): | |
result["headers"] = self.headers | |
else: | |
result["headers"] = {h.key: h.value for h in self.headers} | |
return result | |
def to_dict(self, encode_json=False) -> dict: | |
""" | |
Convert the endpoint to a dictionary, properly handling headers. | |
""" | |
result = super().to_dict(encode_json=encode_json) | |
if self.headers: | |
if isinstance(self.headers, dict): | |
result["headers"] = self.headers | |
else: | |
result["headers"] = {h.key: h.value for h in self.headers} | |
return result |
🧰 Tools
🪛 GitHub Actions: Check - static
[error] 96-96: mypy type checking error: Signature of 'to_dict' incompatible with supertype 'DataClassJsonMixin'. Expected 'to_dict(self, encode_json: Any = ...) -> dict[str, dict[Any, Any] | list[Any] | str | int | float | bool | None]', but found 'to_dict(self) -> dict[Any, Any]'.
🪛 GitHub Actions: Check - lint
[warning] 96-96: pylint W0221: Number of parameters was 2 in 'DataClassJsonMixin.to_dict' and is now 1 in overriding 'Endpoint.to_dict' method (arguments-differ)
🤖 Prompt for AI Agents (early access)
In deepgram/clients/agent/v1/websocket/options.py lines 96 to 107, the to_dict
method signature does not match the parent class DataClassJsonMixin's to_dict,
which expects a parameter encode_json with a default value. Update the method
signature to include encode_json=False and pass this parameter when calling
super().to_dict to ensure compatibility and fix the mypy and pylint errors.
Proposed changes
Proposes a fix for #525
Types of changes
What types of changes does your code introduce to the community Python SDK?
Put an
x
in the boxes that applySummary by CodeRabbit
New Features
Bug Fixes