Integrate the first party YouTube client into Via #1103

jon-betts · 2023-07-19T19:27:24Z

Requires:

Review notes

This includes an enormous commit which is mostly just format changes in the requirements
This is looking up a matching transcript based on a list of preferences
- I don't think we should do this for LMS
- I think the LMS should either do this for us, or make the user pick

Testing notes

make dev
Visit: http://localhost:9083/https://www.youtube.com/watch?v=Xzv84ZdtlE0
You should see a transcript (it should be en, though not sure how you'd tell without a break point)
http://localhost:9083/https://www.youtube.com/watch?v=Xzv84ZdtlE0&via.video.lang=en.a
Should work again
http://localhost:9083/https://www.youtube.com/watch?v=Xzv84ZdtlE0&via.video.lang=en...fr
Should take a while, and potentially timeout. On a repeat attempt you should see it in French

jon-betts · 2023-07-19T19:30:03Z

tests/unit/services.py

-    # By default, the mock reports that URLs are not YouTube URLs.
-    youtube_service.get_video_id.return_value = None
-
-    return youtube_service


This violated the principal of least surprise for me. This is only required in one test, so I moved it there.

jon-betts · 2023-07-19T19:31:04Z

tests/unit/via/views/view_video_test.py

+            "api.youtube.transcript",
+            video_id=youtube_service.get_video_id.return_value,
+            transcript_id="en.a",
+        )


These tests are very intense. I really don't think we should be doing this anyway here which might reduce this effect

jon-betts · 2023-07-19T19:31:58Z

via/views/view_video.py


    if not youtube_service.enabled:
        raise HTTPUnauthorized()

    video_id = youtube_service.get_video_id(url)
-    video_url = youtube_service.canonical_video_url(video_id)
-    video_title = youtube_service.get_video_title(video_id)


I think part of the reason these tests aren't as nice as they could be is we have too many atomic calls.

The new method returns a single object with all these items inside it. It would be good to unify these.

jon-betts · 2023-07-19T19:32:30Z

via/views/view_video.py

    if not video_id:
        raise BadURL(f"Unsupported video URL: {url}", url=url)

-    _, client_config = Configuration.extract_from_params(kwargs)
+    video_url = youtube_service.canonical_video_url(video_id)
+    video_title = youtube_service.get_video_title(video_id)


I moved these down, because we can early return if the video id isn't valid. Rather than doing lots of strange stuff.

jon-betts · 2023-07-19T19:33:47Z

via/views/view_video.py

+        return transcript_id
+
+    # Then look up a transcript matching our preferences
+    video = youtube_service.get_video_info(video_url=url)


We could look this up in the parent, but I chose to keep it separate for now.

We could look to unify the look up with a call like:

youtube_service.get_video_info(video_url=url, get_captions=True)

Where we could use V3 stuff if get_captions=False or the V1 stuff if it's true etc.

seanh · 2023-07-28T11:42:11Z

Labeling this as blocked just because it's waiting for me to get some more urgent PRs done before I can review this. I promise I'm working my way round to reviewing this ASAP!

seanh

Not a proper review yet but just as a first-pass: I think the video_url argument to get_video_info() can be removed as it's unused, and the via.video.lang query param should be removed as nothing's going to be using it

seanh · 2023-08-02T16:19:40Z

via/services/youtube.py

+    def get_video_info(self, video_id=None, video_url=None) -> Video:
+        if video_url:
+            video_id = self.get_video_id(video_url)


Suggested change

def get_video_info(self, video_id=None, video_url=None) -> Video:

if video_url:

video_id = self.get_video_id(video_url)

def get_video_info(self, video_id) -> Video:

The video_url argument isn't needed: this is only called by get_transcript() below and it always passes video_id never video_url

seanh · 2023-08-02T16:21:14Z

via/views/view_video.py

+    # Try the `via.video.lang` param first
+    if transcript_id := via_config.get("video", {}).get("lang"):
+        return transcript_id


Suggested change

# Try the `via.video.lang` param first

if transcript_id := via_config.get("video", {}).get("lang"):

return transcript_id

We should remove this as it'll be dead code: nothing is ever going to be passing the via.video.lang param. We can add support for this query param back in a later PR when we need it

This is mostly arranged acount captions, but has room to grow

This _always_ attempts to get the automatically generated English version. We need to fancy this up to try and find any English track.

seanh · 2023-08-15T11:41:35Z

Closing this, I think it was superseded by #1160

jon-betts added enhancement wip labels Jul 19, 2023

jon-betts self-assigned this Jul 19, 2023

jon-betts commented Jul 19, 2023

View reviewed changes

jon-betts force-pushed the yt-cc-integrate branch 2 times, most recently from 51ae7a8 to 6f8b07a Compare July 19, 2023 19:42

jon-betts changed the base branch from yt-cc-client to recompile-reqs July 19, 2023 19:43

jon-betts force-pushed the yt-cc-integrate branch 3 times, most recently from 87ed39a to 6458ef0 Compare July 20, 2023 17:13

jon-betts force-pushed the recompile-reqs branch from ce105cd to fabdc50 Compare July 20, 2023 17:14

jon-betts force-pushed the yt-cc-integrate branch from 6458ef0 to af7c34e Compare July 20, 2023 17:14

jon-betts force-pushed the recompile-reqs branch from fabdc50 to a0a9d19 Compare July 20, 2023 17:19

jon-betts force-pushed the yt-cc-integrate branch from af7c34e to 70077f4 Compare July 20, 2023 17:21

jon-betts force-pushed the recompile-reqs branch from a0a9d19 to c935e0a Compare July 20, 2023 17:53

jon-betts force-pushed the yt-cc-integrate branch from 70077f4 to bb3c1e3 Compare July 20, 2023 17:54

jon-betts force-pushed the recompile-reqs branch from c935e0a to 2171d2f Compare July 20, 2023 18:19

jon-betts force-pushed the yt-cc-integrate branch from bb3c1e3 to 8bd4b7b Compare July 20, 2023 18:21

jon-betts changed the base branch from recompile-reqs to yt-cc-client July 20, 2023 18:21

jon-betts mentioned this pull request Jul 20, 2023

Unified access to YouTube data via the client #1113

Closed

jon-betts added tech debt and removed wip labels Jul 20, 2023

seanh self-assigned this Jul 21, 2023

jon-betts requested a review from seanh July 24, 2023 15:15

seanh added the blocked label Jul 28, 2023

seanh requested changes Aug 2, 2023

View reviewed changes

Run make requirements with no changes to requirements

aa70824

jon-betts force-pushed the yt-cc-client branch 3 times, most recently from 0a7ed32 to 5ec410d Compare August 2, 2023 19:28

Jon Betts added 4 commits August 2, 2023 20:34

Add a model for parsing YouTube V1 API responses

82533be

This is mostly arranged acount captions, but has room to grow

Add an ID generation mechanism to the YouTube caption objects

61e44ef

Add an algorithm for matching a YouTube caption track

2543c70

Add a client for getting data from YouTube

a507b1e

jon-betts force-pushed the yt-cc-client branch from 5ec410d to a507b1e Compare August 2, 2023 19:36

Jon Betts added 4 commits August 3, 2023 14:24

Use our internal library to retrieve transcripts

72d6d2c

This _always_ attempts to get the automatically generated English version. We need to fancy this up to try and find any English track.

Remove youtube-transcript-api requirement

717c664

Add h-matchers to the dependencies for prod

68765af

If no transcript is provided, search for a consistent transcript

e994778

jon-betts force-pushed the yt-cc-integrate branch from 8bd4b7b to e994778 Compare August 3, 2023 13:24

jon-betts force-pushed the yt-cc-client branch from a507b1e to afc94db Compare August 3, 2023 14:06

seanh removed the blocked label Aug 4, 2023

seanh closed this Aug 15, 2023

seanh deleted the yt-cc-integrate branch August 15, 2023 11:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate the first party YouTube client into Via #1103

Integrate the first party YouTube client into Via #1103

jon-betts commented Jul 19, 2023 •

edited

Loading

jon-betts Jul 19, 2023

jon-betts Jul 19, 2023 •

edited

Loading

jon-betts Jul 19, 2023

jon-betts Jul 19, 2023

jon-betts Jul 19, 2023

seanh commented Jul 28, 2023

seanh left a comment

seanh Aug 2, 2023

seanh Aug 2, 2023 •

edited

Loading

seanh commented Aug 15, 2023 •

edited

Loading

	# Try the `via.video.lang` param first
	if transcript_id := via_config.get("video", {}).get("lang"):
	return transcript_id

Integrate the first party YouTube client into Via #1103

Integrate the first party YouTube client into Via #1103

Conversation

jon-betts commented Jul 19, 2023 • edited Loading

Review notes

Testing notes

jon-betts Jul 19, 2023

Choose a reason for hiding this comment

jon-betts Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

jon-betts Jul 19, 2023

Choose a reason for hiding this comment

jon-betts Jul 19, 2023

Choose a reason for hiding this comment

jon-betts Jul 19, 2023

Choose a reason for hiding this comment

seanh commented Jul 28, 2023

seanh left a comment

Choose a reason for hiding this comment

seanh Aug 2, 2023

Choose a reason for hiding this comment

seanh Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

seanh commented Aug 15, 2023 • edited Loading

jon-betts commented Jul 19, 2023 •

edited

Loading

jon-betts Jul 19, 2023 •

edited

Loading

seanh Aug 2, 2023 •

edited

Loading

seanh commented Aug 15, 2023 •

edited

Loading