Set up an endpoint for meeting rooms #1693
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report
@@ Coverage Diff @@
## master #1693 +/- ##
===================================================
+ Coverage 53.02876% 53.09194% +0.06318%
===================================================
Files 74 75 +1
Lines 4903 4916 +13
Branches 976 977 +1
===================================================
+ Hits 2600 2610 +10
- Misses 1970 1972 +2
- Partials 333 334 +1
Continue to review full report in Codecov by Sentry.
|
| } | ||
|
|
||
| const id = uuid(); | ||
| const svc = new RoomServiceClient( |
There was a problem hiding this comment.
Is it ok to create one of these on every request? If there are definite gains in having a single instance lmk and we can work sth out, like putting it on the req object as well
There was a problem hiding this comment.
Yeah I wondered that too, when I asked livekit they said it should be ok:
either way works! it's pretty inexpensive to create each time
Sound ok to you or should we press further?
| emptyTimeout: 30 * 60, | ||
| maxParticipants: 2, | ||
| }; | ||
| const room = await svc.createRoom(opts); |
There was a problem hiding this comment.
Do we not need to create a stream anymore? So there will be a separate call to have the room be livestreamed, is that it?
There was a problem hiding this comment.
That's right, since Hunter mentioned that not every room will have a live stream i've kept it separate.
| /room: | ||
| post: | ||
| description: create a new room | ||
| responses: |
There was a problem hiding this comment.
Since you're adding it here, mind documenting the success respnose schema? can just do an inline schema here with an id field etc
| import { RoomServiceClient } from "livekit-server-sdk"; | ||
| jest.mock("livekit-server-sdk"); | ||
| const MockedRoomServiceClient = | ||
| RoomServiceClient as jest.Mock<RoomServiceClient>; |
There was a problem hiding this comment.
Maybe these should be the first lines in this file, just in case anyone changes the impl to use the mocked stuff statically (or this is really magic and I don't know how it works)
There was a problem hiding this comment.
What do you mean by using the mocked stuff statically?
There was a problem hiding this comment.
e.g.: if we moved the livekit client creation to the "file root" or server creation level, not creating one on every request
That could run before the mocks are setup and get the wrong result. Not a huge priority since it works now though, it's fine as is
* Set up an endpoint for meeting rooms * Add unit test * fix test
What does this pull request do? Explain your changes. (required)
Provides a new endpoint to be able to create meeting rooms for multi-participant live streams.
How did you test each of these updates (required)
Ran the app locally with
yarn devand tested with different requests to the new endpoint. There is also a unit test provided.Does this pull request close any open issues?
Checklist