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

[ie/adn] require subscriptions in German #9068

Merged
merged 7 commits into from Jan 28, 2024

Conversation

infanf
Copy link
Contributor

@infanf infanf commented Jan 25, 2024

ADN DE requires an account and subscription for all videos
add error message for missing subscription case if logged in

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

ADN only supports login using username/password. Adjust raise_login_required call accordingly.
Also the German site is still somewhat restricted as it doesn't provide free ad supported content (yet). Added checks for this and raise Exceptions if the user isn't logged in or doesn't have a subcription.

Fixes #9067

Template

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 all of the following options that apply:

  • 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?

ADN DE requires an account and subscription for all videos
add error message for missing subscription case if logged in
@infanf infanf changed the title fix(#9067): adjust error messages [ie/adn] adjust error messages Jan 25, 2024
@infanf infanf changed the title [ie/adn] adjust error messages [ie/adn] require subscriptions in German Jan 25, 2024
@bashonly bashonly added the site-bug Issue with a specific website label Jan 26, 2024
yt_dlp/extractor/adn.py Outdated Show resolved Hide resolved
yt_dlp/extractor/adn.py Outdated Show resolved Hide resolved
yt_dlp/extractor/adn.py Outdated Show resolved Hide resolved
@bashonly bashonly added the pending-fixes PR has had changes requested label Jan 27, 2024
yt_dlp/extractor/adn.py Outdated Show resolved Hide resolved
yt_dlp/extractor/adn.py Outdated Show resolved Hide resolved
Comment on lines 185 to 189
video = options['video']
startDate = video.get('startDate')
currentDate = video.get('currentDate')
if startDate and currentDate and startDate > currentDate:
raise ExtractorError(f'This video is not available yet. Release date: {startDate}', expected=True)
Copy link
Member

@bashonly bashonly Jan 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these values unix timestamps? And if so, should we rely on currentDate instead of time.time()? IMO we should do this a bit more safely (and use snake_case):

Suggested change
video = options['video']
startDate = video.get('startDate')
currentDate = video.get('currentDate')
if startDate and currentDate and startDate > currentDate:
raise ExtractorError(f'This video is not available yet. Release date: {startDate}', expected=True)
start_date = traverse_obj(options, ('video', 'startDate', {int}))
current_date = traverse_obj(options, ('video', 'currentDate', {int}))
if start_date and current_date and start_date > current_date:
raise ExtractorError(f'This video is not available yet. Release date: {start_date}', expected=True)

and if these are unix timestamps, Release date: {start_date} is not useful IMO

Perhaps we should even check this after the if not formats: statement below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are ISO 8601 UTC timestamps. Do I need to convert that to unix timestamps or datetime for comparison?
If this is a future video we won't reach the formats section, because user.get('hasAccess') will be false.
Thanks for pointing out snake case, that's what you get for relying on Copilot too much 🤦
I'll push a fix tomorrow.

Copy link
Member

@bashonly bashonly Jan 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok. I'd import parse_iso8601 from utils and import time and do this then:

Suggested change
video = options['video']
startDate = video.get('startDate')
currentDate = video.get('currentDate')
if startDate and currentDate and startDate > currentDate:
raise ExtractorError(f'This video is not available yet. Release date: {startDate}', expected=True)
start_date = traverse_obj(options, ('video', 'startDate', {str}))
if (parse_iso8601(start_date) or 0) > time.time():
raise ExtractorError(f'This video is not available yet. Release date: {start_date}', expected=True)

.gitignore Outdated
@@ -47,6 +47,7 @@ cookies
*.png
*.sbv
*.srt
*.ssa
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will add this in the cleanup commit instead

Suggested change
*.ssa

yt_dlp/extractor/adn.py Outdated Show resolved Hide resolved
@bashonly bashonly self-assigned this Jan 28, 2024
@bashonly bashonly removed the pending-fixes PR has had changes requested label Jan 28, 2024
@bashonly bashonly merged commit 9526b1f into yt-dlp:master Jan 28, 2024
6 checks passed
@infanf infanf deleted the fix/adn-de-subscription branch January 28, 2024 17:36
FletcherD pushed a commit to FletcherD/yt-dlp that referenced this pull request Feb 14, 2024
aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
site-bug Issue with a specific website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

German ADN Site doesn't work with cookies
2 participants