-
Notifications
You must be signed in to change notification settings - Fork 104
Fix ingress creation parameter inline documentation #296
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
Changes from all commits
6d72a96
6e79d67
c910313
e293ec5
8622d73
2fb35a6
b42ce93
681011e
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 @@ | ||
| --- | ||
| "livekit-server-sdk": patch | ||
| --- | ||
|
|
||
| Fix ingress creation parameter inline documentation |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -22,13 +22,13 @@ export interface CreateIngressOptions { | |||||
| */ | ||||||
| name?: string; | ||||||
| /** | ||||||
| * name of the room to send media to. optional | ||||||
| * name of the room to send media to. required | ||||||
| */ | ||||||
| roomName?: string; | ||||||
| /** | ||||||
| * unique identity of the participant. optional | ||||||
| * unique identity of the participant. required | ||||||
| */ | ||||||
| participantIdentity?: string; | ||||||
| participantIdentity: string; | ||||||
| /** | ||||||
| * participant display name | ||||||
| */ | ||||||
|
|
@@ -67,11 +67,11 @@ export interface UpdateIngressOptions { | |||||
| */ | ||||||
| name: string; | ||||||
| /** | ||||||
| * name of the room to send media to. optional | ||||||
| * name of the room to send media to. | ||||||
| */ | ||||||
| roomName?: string; | ||||||
| /** | ||||||
| * unique identity of the participant. optional | ||||||
| * unique identity of the participant. | ||||||
| */ | ||||||
| participantIdentity?: string; | ||||||
| /** | ||||||
|
|
@@ -134,29 +134,35 @@ export class IngressClient extends ServiceBase { | |||||
| * @param inputType - protocol for the ingress | ||||||
| * @param opts - CreateIngressOptions | ||||||
| */ | ||||||
| async createIngress(inputType: IngressInput, opts?: CreateIngressOptions): Promise<IngressInfo> { | ||||||
| async createIngress(inputType: IngressInput, opts: CreateIngressOptions): Promise<IngressInfo> { | ||||||
| let name: string = ''; | ||||||
| let roomName: string = ''; | ||||||
| let participantName: string = ''; | ||||||
| let participantIdentity: string = ''; | ||||||
| let participantMetadata: string | undefined; | ||||||
| let bypassTranscoding: boolean = false; | ||||||
| let enableTranscoding: boolean | undefined; | ||||||
| let url: string = ''; | ||||||
| let audio: IngressAudioOptions | undefined; | ||||||
| let video: IngressVideoOptions | undefined; | ||||||
|
|
||||||
| if (opts !== undefined) { | ||||||
| name = opts.name || ''; | ||||||
| roomName = opts.roomName || ''; | ||||||
| participantName = opts.participantName || ''; | ||||||
| participantIdentity = opts.participantIdentity || ''; | ||||||
| bypassTranscoding = opts.bypassTranscoding || false; | ||||||
| enableTranscoding = opts.enableTranscoding; | ||||||
| url = opts.url || ''; | ||||||
| audio = opts.audio; | ||||||
| video = opts.video; | ||||||
| participantMetadata = opts.participantMetadata; | ||||||
| if (opts == null) { | ||||||
| throw new Error('options dictionary is required'); | ||||||
| } | ||||||
|
|
||||||
| const roomName: string | undefined = opts.roomName; | ||||||
| const enableTranscoding: boolean | undefined = opts.enableTranscoding; | ||||||
| const audio: IngressAudioOptions | undefined = opts.audio; | ||||||
| const video: IngressVideoOptions | undefined = opts.video; | ||||||
| const participantMetadata: string | undefined = opts.participantMetadata; | ||||||
|
|
||||||
| name = opts.name || ''; | ||||||
|
Member
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. need consts in front of these |
||||||
| participantName = opts.participantName || ''; | ||||||
| participantIdentity = opts.participantIdentity || ''; | ||||||
|
Member
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.
Suggested change
Contributor
Author
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. I also want to fail if the parameter is specifically set, but to an empty string. Is there a better way to achieve this (beyond 2 tests in the if statement below)? |
||||||
| bypassTranscoding = opts.bypassTranscoding || false; | ||||||
| url = opts.url || ''; | ||||||
|
|
||||||
| if (typeof roomName == 'undefined') { | ||||||
| throw new Error('required roomName option not provided'); | ||||||
| } | ||||||
|
|
||||||
| if (participantIdentity == '') { | ||||||
| throw new Error('required participantIdentity option not provided'); | ||||||
| } | ||||||
|
|
||||||
| const req = new CreateIngressRequest({ | ||||||
|
|
||||||
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.
this is optional right? on updates.
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.
Everything is optional in the dictionary in the sense that parameters left out will not be updated. It's odd to single out these 2 parameters. We could flag all parameters as optional, but this seems redundant on a parameter called "UpdateIngressOptions".
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.
got it.. makes sense.