-
Notifications
You must be signed in to change notification settings - Fork 110
Alex/adding display name #1216
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
Alex/adding display name #1216
Changes from all commits
ad29c65
770fc8d
46aacaf
c01fdae
fe836b0
b30e015
a903bff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "github.com/livekit/protocol": minor | ||
| --- | ||
|
|
||
| Added DisplayName field to CreateSIPParticipantRequest |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,7 +105,15 @@ message InternalCreateSIPParticipantRequest { | |
| // Wait for the answer for the call before returning. | ||
| bool wait_until_answered = 29; | ||
|
|
||
| // NEXT ID: 31 | ||
| // Optional display name for the 'From' SIP header. | ||
| // | ||
| // Cases: | ||
| // 1) Unspecified: Use legacy behavior - display name will be set to be the caller's number. | ||
| // 2) Empty string: Do not send a display name, which will result in a CNAM lookup downstream. | ||
| // 3) Non-empty: Use the specified value as the display name. | ||
| optional string display_name = 31; | ||
|
|
||
| // NEXT ID: 32 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dennwc perhaps for another change, but couldn't we reuse CreateSIPParticipantRequest here ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not directly, but yes, this is on my TODO list as well. Sharing one structure likely won't work well, because internal has a different representation of a few things (e.g. feature flags). I was thinking that we should slice this structure into a few smaller ones. Everything related to media setup can be one structure, shared for internal Create, public Create and also for inbound trunks. Similarly, there are fields related to how SIP should join LK room, this could be another structure shared for Create and EvaluateDispatch. The plan is also to allow storing these objects directly into the DB, so that we don't have to flatten/unflatten from SQL and add columns each time we add another flag that is never used as a filter for DB queries. |
||
| } | ||
|
|
||
| message InternalCreateSIPParticipantResponse { | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
In theory, we could avoid
optionalhere, since we can populatedisplay_nameto a number inNewCreateSIPParticipantRequest. The only concern would be that the backend still cannot know if it should follow the old logic or a new one.