Implemented Twilio Video Api #20
Conversation
|
Hey, this is great! Can you run Thanks! |
|
Hey, done it. |
| t.Parallel() | ||
| client, server := getServer(videoRecordingResponse) | ||
| defer server.Close() | ||
| recording, err := client.Video.VideoRecordings.Get(context.Background(), "RT63868a235fc1cf3987e6a2b67346273f") |
There was a problem hiding this comment.
From an API perspective there's a little stutter here where we write Video twice. Would it be better as client.Video.Recordings.Get()?
(Or that would overlap with the existing Recordings struct, huh. Ugh.)
There was a problem hiding this comment.
I saw that but I wasn't sure what to make with it. Also I find that when using Client it's easy to make null pointer dereference.
To solve that I think in this place in Client declaration I should remove Rooms and VideoRecordings and make some VideoClient struct with Client and this two services
type VideoClient struct {
*Client
Rooms *RoomService
Recordings *VideoRecordingService
}
Also I think it would be good for other services too, what you think?
There was a problem hiding this comment.
Maybe for a separate pull request?
There was a problem hiding this comment.
so I should leave this VideoClient as it is now, or you mean to move into seperate pull request changing all other Clients and here change only VideoClient?
| // When you make a request to this URL, Twilio will generate a temporary URL for accessing | ||
| // this binary data, and issue an HTTP 302 redirect response to your request. The Recording | ||
| // will be returned in the format as described in the metadata. | ||
| func (vr *VideoRecordingService) Media(ctx context.Context, sid string) (*VideoMedia, error) { |
There was a problem hiding this comment.
Would it make sense to have the API resemble this one for Messages Media? https://github.com/kevinburke/twilio-go/blob/master/media.go#L59
There was a problem hiding this comment.
I saw that but are you sure we need that to download this media? Because when i was looking at this Messages Media it was looking like some complicated stuff (https://www.twilio.com/docs/api/messaging/media#media-list-resource) but in VideoRecordings they returning proper link to download it
{
"location": "https://com.twilio.dev-us1.video.recording.s3.amazonaws.com/RTXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX"
}
|
Merged as 8f07b10, thanks! |
This PR implements Twilio Video Api #1 : https://www.twilio.com/docs/api/video/rest
It incluedes implementation for two services
Room- https://www.twilio.com/docs/api/video/rooms-resourceRecordings- https://www.twilio.com/docs/api/video/recordings-resource