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

Discogs Add extractor Revised #6881

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

elanrosen
Copy link

@elanrosen elanrosen commented Apr 21, 2023

Description of your pull request and other information

Added several changes and additions to the discogs extractor, including error handling, metadata extraction, and checks for DRM-protected and potentially infringing content. It now uses a try-except block to handle exceptions, checks whether the API response contains valid data, and examines the tracklist and media items in the API response to extract additional metadata. The code also uses the sanitize_filename function to generate a safe filename and passes additional metadata to the playlist_result method.

Improvements to #6624

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?

@bashonly
Copy link
Member

bashonly commented Apr 21, 2023

The original PR is still pretty recent, with review comments from only a week ago.

The review comments from the original PR have not been addressed in this PR.

And a lot of the code added in this PR seems pointless or doesn't make sense, e.g. error handling, URL checks, DRM check (all of the video URLs returned by the Discogs API are Youtube links.)

The extra playlist metadata could be useful, but the new metadata extraction code does not follow the project's conventions. Most of it also fails to account for compilations and other "various artists"-type releases.

@pukkandan pukkandan force-pushed the master branch 2 times, most recently from ee280c7 to 7aeda6c Compare May 24, 2023 18:09
@pukkandan pukkandan added site-request Request to support a new website pending-fixes PR has had changes requested stale-pr PR that has been pending fixes for a long time and removed do-not-merge labels May 29, 2023
@bashonly
Copy link
Member

Original discogs extractor PR was merged in 6daaf21

@pukkandan pukkandan closed this Jun 15, 2023
@pukkandan pukkandan reopened this Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending-fixes PR has had changes requested site-request Request to support a new website stale-pr PR that has been pending fixes for a long time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants