add trunk_config to WarmTransferTask for SIP endpoint transfers#5016
add trunk_config to WarmTransferTask for SIP endpoint transfers#5016
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| if self._sip_trunk_id is None and self._trunk_config is None: | ||
| raise ValueError( | ||
| "`LIVEKIT_SIP_OUTBOUND_TRUNK` environment variable or `sip_trunk_id` argument must be set" | ||
| "`LIVEKIT_SIP_OUTBOUND_TRUNK` environment variable, `sip_trunk_id`, or `trunk_config` must be set" | ||
| ) |
There was a problem hiding this comment.
🟡 Validation no longer rejects empty-string sip_trunk_id, allowing invalid API calls
The old validation used if not self._sip_trunk_id: which rejected both None and empty string "". The new code at line 112 uses if self._sip_trunk_id is None and self._trunk_config is None:, which only rejects None. This means an empty-string sip_trunk_id (e.g. from LIVEKIT_SIP_OUTBOUND_TRUNK="" in the environment, or from an explicit sip_trunk_id="" argument) now passes validation and is forwarded to CreateSIPParticipantRequest at warm_transfer.py:309, where it will cause a confusing server-side error instead of the clear ValueError the old code provided.
| if self._sip_trunk_id is None and self._trunk_config is None: | |
| raise ValueError( | |
| "`LIVEKIT_SIP_OUTBOUND_TRUNK` environment variable or `sip_trunk_id` argument must be set" | |
| "`LIVEKIT_SIP_OUTBOUND_TRUNK` environment variable, `sip_trunk_id`, or `trunk_config` must be set" | |
| ) | |
| if not self._sip_trunk_id and self._trunk_config is None: | |
| raise ValueError( | |
| "`LIVEKIT_SIP_OUTBOUND_TRUNK` environment variable, `sip_trunk_id`, or `trunk_config` must be set" | |
| ) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| *, | ||
| hold_audio: NotGivenOr[AudioSource | AudioConfig | list[AudioConfig] | None] = NOT_GIVEN, | ||
| sip_trunk_id: NotGivenOr[str] = NOT_GIVEN, | ||
| trunk_config: NotGivenOr[api.SIPOutboundConfig] = NOT_GIVEN, |
There was a problem hiding this comment.
I'm not the biggest fan of the trunk terminology.. because a user might just want to dial out to another sip address. we were talking about changing this in the CreateSIPParticipant API as well.
wdyt about simplifying it like the following:
- rename sip_number to sip_user (it's the same thing, user is the more technically correct term)
- map to deprecated sip_number field
- allow user to pass in: sip_trunk_id or sip_connection: SIPConnection
SIPConnection would contain:
- address
- username
- password
- destination_country: str
There was a problem hiding this comment.
another idea.. thinking out loud.. this could be made even more generic.. where we could work with our Whatsapp and Twilio connectors too.. if we refactored out the dial out portion into a function.. then it could be
SIPDialAction(sip_user, address, username, password, destination_country)SIPTrunkDialAction(sip_user, trunk_id)TwilioConnectorDial(twilio_api_key, to_number, etc)WhatsappConnectorDial(...)
There was a problem hiding this comment.
is there a reference for the Whatsapp and Twilio connectors?
2 participants
No description provided.