Closed Einsight04 closed 1 year ago
agreed it would be nice to support all of these parameters - can we achieve this with a **kwargs
somewhere instead? don't want to bloat TwilioConfig
agreed it would be nice to support all of these parameters - can we achieve this with a
**kwargs
somewhere instead? don't want to bloatTwilioConfig
Although **kwargs
are indeed possible when using Pydantic, it's worth noting that static type checkers like Pylance will continue to flag extra parameters that are not explicitly declared in the model.
By declaring optional parameters directly in the TwilioConfig model, we can avoid these warnings and also benefit from auto-completion.
If we still want to lessen the bloat, we can consider having an extra parameter in the model that accepts a dictionary for any additional fields. Like so:
class TwilioConfig(BaseModel):
account_sid: str
auth_token: str
record: bool = False
extra: Optional[Dict[str, Any]] = None
This keeps the model's interface clean and allows some degree of flexibility, while still benefiting from Pydantic's validation for the main fields.
Let me know how you'd like to approach this and ill pr.
@Einsight04 i'm ok with either approach (as long as the vars are kept in a dict)
looks like @KShah707 took a stab at this in #271 – happy to merge once ready!
This 'extra' method seems nice, decouples the Vocode library from minor changes in the underlying Twilio api (as we don't need to update TwilioConfig if fields change). I think it's the best compromise but will sleep on it
Duplicate merged in #274
When creating a call with vocode we can only customize the following attributes:
But Twillio allows for much more customization that Vocode misses out on.
Current TwilioConfig:
Propsed TwilioConfig:
Putting this into use only requires updating the parameters passed into Twillio's
create()
method and is overall a very minor yet impactful change.