Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
NBC site broken #22693
NBC site broken #22693
Comments
|
The original poster's URL works for me, but this one does not and gives the same log as he posted http://www.nbc.com/saturday-night-live/video/october-12-david-harbour/4046108 |
|
I ran into this this morning. The index error is from this:
That is, the youtube-dl/youtube_dl/extractor/nbc.py Lines 94 to 109 in c317b61 For an example like this SNL URL, we know the
so far so good but:
|
|
I found I could get a non-empty response for some videos by using
api.nbc.com/v3.14/videos instead of api.nbc.com/v3/videos
|
|
GOOD EYE, @raleeper! Confirmed. That seems…really weird. https://api.nbc.com/ lists the API versions, and oddly enough I had tried It appears that I'm really at a loss to understand a rational basis for this behavior. Surely 2 through 14 have been around for a while, and why would they tinker with the functionality of 0 through 1? Maybe the most plausible theory is somehow the mapping of The below patch certainly makes it work, but it doesn't seem like the best solution? On the other hand, forcing a roundtrip to check for the latest (I had also spent awhile trying to figure out how to make the diff --git a/youtube_dl/extractor/nbc.py b/youtube_dl/extractor/nbc.py
index 3282f84ee..49c987320 100644
--- a/youtube_dl/extractor/nbc.py
+++ b/youtube_dl/extractor/nbc.py
@@ -85,7 +85,7 @@ class NBCIE(AdobePassIE):
permalink, video_id = re.match(self._VALID_URL, url).groups()
permalink = 'http' + compat_urllib_parse_unquote(permalink)
response = self._download_json(
- 'https://api.nbc.com/v3/videos', video_id, query={
+ 'https://api.nbc.com/v3.14/videos', video_id, query={
'filter[permalink]': permalink,
'fields[videos]': 'description,entitlement,episodeNumber,guid,keywords,seasonNumber,title,vChipRating',
'fields[shows]': 'shortTitle', |
|
There's also a time element here. As others have reported, some videos return empty data using the v3 api when they are very new (like morning after airing). The data appears correctly later in the day. Seems like some sort of timezone issue in v3 that is fixed in v3.14. That explains why the issue has been hard to re-create after the fact. It's a mystery why v3 isn't an alias for the latest v3.x. |
|
So, PR with the above patch (and perhaps an explanatory comment, although I've been slapped down for those before)? Or something else? |
|
Oh, also, do we want better error handling for this situation ("No metadata returned by API call."), or is "IndexError: list index out of range" ok? |
|
Right now it is an uncaught exception, a customzied error to help guide the user would be better. |
|
BTW, I could have swore I came back here this morning to post the update that the original cited video started working this morning. I even clicked the "close and comment". I assumed something was broken on the provider side that was since fixed, but it sounds like more is at foul here. |
|
I would go with the simplest possible change. If you aren't a python programmer, any message about a situation that you can't fix by changing command line parameters is the same as an uncaught exception. Both mean "extractor broken." |
Although it's true that there's not much difference from the user's perspective, and we don't want to discourage users from reporting an Issue with a message like "This will probably work if you wait a day and try tomorrow," it's also true that many users are python programmers and giving them an idea of what the problem is that doesn't involve pulling out a debugger means it's far more likely that someone will take a look at the problem and either craft a solution or at least produce a bug report with some analysis. That said, without some guidance as to what kind of checking and reporting would be most appropriate, I'm not at all sure what to do. At a minimum we could break I would probably test for In any event, this could be a separate PR. Perhaps the committers could offer some guidance assuming they merge #22701. |
|
This is just a temporary work-around for downloading NBC and NBC News videos until the programmers update the YT-DL code. This worked for me in downloading both today's and last week's Meet The Press and last night's SNL. Using the following as a template https://player.theplatform.com/p/HNK2IC/uW4uIUm_KHR6/select/media/guid/2410887629/xxxxxxx replace the seven x's with the number of the video to be downloaded. The video's number, currently, should begin with "404." (The "404" has nothing to do with a 404 webpage.) To get the video number, refer to the URL of the webpage containing the video. Today's Meet The Press is: https://www.nbc.com/meet-the-press/video/meet-the-press-101319/4047204 Note that the last number is 4047204 Replacing the 7 x's in the template with 4047204, we have: https://player.theplatform.com/p/HNK2IC/uW4uIUm_KHR6/select/media/guid/2410887629/4047204 The above URL will download (at the moment) in YT-DL. This may not last but might be helpful until the programmers update YT-DL. |
Thank you @bonacker1 !! This worked for me! |
|
@remitamine: I hesitate to open a new issue, but as of a few minutes ago,
but I tried an old checkout of 2017.11.06 and it worked fine. This suggests to me that graphql may not have been the right choice for all situations. |
|
not accessible for me in the browser.
|
Agreed. But it is linked[*] from https://www.nbc.com/saturday-night-live/episodes and present in the v3.14 API and downloadable with older youtube-dl versions. Edit: [*] well, it's some kind of a javascript click event handler rather than a link per se, but... |
|
I see this as a problem with the website, not the use of GraphQL API. |
Checklist
Verbose log
Description
Latest NBC dateline episode plays via browser but Index Error via youtube-dl
https://www.nbc.com/dateline/video/the-plan/4040189