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

[youporn] Fix extraction of metadata #2768

Merged
merged 3 commits into from Nov 26, 2022
Merged

Conversation

marieell
Copy link
Contributor

Before submitting a pull request make sure you have:

In order to be accepted and merged into yt-dlp 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

Closes #2701. Youporn sends a JS challenge with TLS1.3-

yt_dlp/extractor/youporn.py Outdated Show resolved Hide resolved
@pukkandan pukkandan added NSFW site-bug Issue with a specific website enhancement New feature or request and removed enhancement New feature or request labels Feb 14, 2022
@pukkandan
Copy link
Member

pukkandan commented May 7, 2022

Outdated
  1. The idea of creating a new context is interesting, but more consideration must be given for how it is implemented. (Related: [Feature Request]Ability to use a separate file which stores names of cookie files to use and either extractor/domain #258, [options] Add --legacy-server-connect #778, [ceskatelevize.cz] Cannot download manifest - SSLV3_ALERT_HANDSHAKE_FAILURE #2043). For the time-being, implement PROTOCOL_TLSv1_2 into --legacy-server-connect instead. You can catch the relevant error in the extractor and ask user to apply this option.
  2. Why use context.post_handshake_auth? afaik, it is not applicable for us

  1. Use merge_dicts instead of all if not data[...]: data[...] = ... in extractor code. The same can be used for the final update

@pukkandan pukkandan added the pending-fixes PR has had changes requested label May 7, 2022
@coletdjnz

This comment was marked as outdated.

@pukkandan pukkandan force-pushed the master branch 2 times, most recently from a63ff77 to b14d523 Compare May 18, 2022 03:35
@pukkandan
Copy link
Member

#2701 has been fixed in a different way, so only the site enhancements is remaining from this PR

@pukkandan pukkandan added site-enhancement Feature request for some website and removed enhancement New feature or request site-bug Issue with a specific website labels Jun 11, 2022
@pukkandan pukkandan added the stale-pr PR that has been pending fixes for a long time label Jun 22, 2022
@marieell marieell force-pushed the youporn branch 2 times, most recently from 2481be8 to 85faa13 Compare November 25, 2022 17:35
@marieell
Copy link
Contributor Author

3. Use merge_dicts instead of all if not data[...]: data[...] = ... in extractor code. The same can be used for the final update

But that would always execute all these extractions, even if they are not necessary?

@pukkandan
Copy link
Member

Is that an issue?

@marieell
Copy link
Contributor Author

It's a lot of possibly unneeded string matching for in my opinion not much of a style improvement. Well. What do you want it to look like?

data = merge_dicts(data, { 'title': self._html_search_regex(
// …
)})

@pukkandan
Copy link
Member

Ive pushed some changes. pls add a test

@pukkandan
Copy link
Member

Actually, I just realized that the whole PR is really just one line. Everything else is unnecessary changes!

@marieell
Copy link
Contributor Author

Alright, I squashed the commits.

@pukkandan pukkandan removed the stale-pr PR that has been pending fixes for a long time label Nov 26, 2022
@pukkandan
Copy link
Member

That was not necessary. Pls avoid force pushing in future

pls add a test

?

@marieell
Copy link
Contributor Author

Pls avoid force pushing in future

Is that documented somewhere? Most projects I know strongly encourage cleaning up commits in PRs.

@pukkandan pukkandan removed the pending-fixes PR has had changes requested label Nov 26, 2022
yt_dlp/extractor/youporn.py Outdated Show resolved Hide resolved
@pukkandan pukkandan merged commit 86f557b into yt-dlp:master Nov 26, 2022
@pukkandan
Copy link
Member

Is that documented somewhere?

No

Most projects I know strongly encourage cleaning up commits in PRs.

It makes reviewing harder. And we squash and merge anyway

@marieell marieell deleted the youporn branch November 26, 2022 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NSFW site-enhancement Feature request for some website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[YouPorn] Unable to download JSON metadata: HTTP Error 403: Forbidden
4 participants