Skip to content
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

[canvas] fix gone error vrtnu. fixes #26957 #27053

Closed
wants to merge 3 commits into from
Closed

[canvas] fix gone error vrtnu. fixes #26957 #27053

wants to merge 3 commits into from

Conversation

bendekeersm
Copy link

@bendekeersm bendekeersm commented Nov 17, 2020

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

Explanation of your pull request in arbitrary form goes here. Please make sure the description explains the purpose and effect of your pull request and is worded well enough to be understood. Provide as much context and examples as possible.

@bendekeersm
Copy link
Author

fixes #26957

@Jan-PieterBaert
Copy link

I have checked this with https://www.vrt.be/vrtnu/a-z/de-ideale-wereld/2020-nj/de-ideale-wereld-d20201117/ and it works fine locally 😃

@JensTimmerman JensTimmerman mentioned this pull request Nov 18, 2020
9 tasks
@cypheron
Copy link

cypheron commented Nov 19, 2020

You should re-trigger the CI by force pushing your code unchanged (commit --amend first). Travis CI is showing jobs failed even if they didn't run, like in my case and also yours.

@bendekeersm
Copy link
Author

You should re-trigger the CI by force pushing your code unchanged (commit --amend first). Travis CI is showing jobs failed even if they didn't run, like in my case and also yours.

Thanks for pointing that out. Was my very first pull request, so a bit uncertain if I did something wrong or not. Will check it later today

@ottob
Copy link

ottob commented Nov 21, 2020

This did not work for me:

% python -m youtube_dl https://www.vrt.be/vrtnu/a-z/wanderlust/1/wanderlust-s1a2/
[VrtNU] wanderlust-s1a2: Downloading webpage
[VrtNU] vid-22d8766f-ec31-4ffe-b4aa-3682f46db7ce: Downloading token
[VrtNU] vid-22d8766f-ec31-4ffe-b4aa-3682f46db7ce: Downloading video JSON
WARNING: Unable to download JSON metadata: HTTP Error 400: Bad Request
Traceback (most recent call last):
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 174, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/Users/ottobretz/Downloads/youtube-dl-master/youtube_dl/__main__.py", line 19, in <module>
    youtube_dl.main()
  File "youtube_dl/__init__.py", line 474, in main
    _real_main(argv)
  File "youtube_dl/__init__.py", line 464, in _real_main
    retcode = ydl.download(all_urls)
  File "youtube_dl/YoutubeDL.py", line 2029, in download
    url, force_generic_extractor=self.params.get('force_generic_extractor', False))
  File "youtube_dl/YoutubeDL.py", line 796, in extract_info
    return self.__extract_info(url, ie, download, extra_info, process)
  File "youtube_dl/YoutubeDL.py", line 803, in wrapper
    return func(self, *args, **kwargs)
  File "youtube_dl/YoutubeDL.py", line 824, in __extract_info
    ie_result = ie.extract(url)
  File "youtube_dl/extractor/common.py", line 532, in extract
    ie_result = self._real_extract(url)
  File "youtube_dl/extractor/canvas.py", line 375, in _real_extract
    for target in data['targetUrls']:
TypeError: 'bool' object has no attribute '__getitem__'

@Jan-PieterBaert
Copy link

@ottob it works for me when using the netrc file, but not without, it's probably something you need a token for

@ottob
Copy link

ottob commented Nov 21, 2020

That was it, thanks. I updated my netrc and now it works with this patch.

@ghen2
Copy link

ghen2 commented Nov 26, 2020

Same here, but it used to fail gracefully, with a clear error indicating the need to authenticate, instead of just HTTP 400: Bad request.

When authenticated it's working fine with this patch, thanks for that!

@editicalu
Copy link

Can confirm that this fixes 410 Gone.

@doxxuni
Copy link

doxxuni commented Dec 18, 2020

Tested and working

@ghen2
Copy link

ghen2 commented Dec 30, 2020

Hi
What is needed to get this merged besides 6 positive reviews?
This affects only a single extractor, that has been broken for >2 months now (#26957).

@bendekeersm
Copy link
Author

Hi
What is needed to get this merged besides 6 positive reviews?
This affects only a single extractor, that has been broken for >2 months now (#26957).

Good question. It was my first time contributing. I expected creating a pull request would be enough to get it merged.

@Jan-PieterBaert
Copy link

If you can, you could try to ask a review from one of the maintainers

@MatthiasCoppens
Copy link

@bendekeersm People on IRC said to clean the commented out code first. If no-one reviews it after those changes, you could maybe ping the maintainers on IRC (#youtube-dl on Freenode).

@remitamine remitamine closed this in 1ae7ae0 Jan 5, 2021
ThirumalaiK pushed a commit to ThirumalaiK/youtube-dl that referenced this pull request Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants