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

[networking] Remove _CompatHTTPError #8871

Merged

Conversation

coletdjnz
Copy link
Member

IMPORTANT: PRs without the template will be CLOSED

Description of your pull request and other information

_CompatHTTPError was to help with transition to the networking framework. It is not used in yt-dlp, but was there in the rare case plugins or external scripts were catching it.

These days it should be safe to remove. It just bloats the logs we get from users.

Be sure to add a breaking change announcement on the release changelog.

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?

@pukkandan pukkandan self-assigned this Dec 29, 2023
@coletdjnz coletdjnz added the networking core networking related label Dec 29, 2023
@pukkandan
Copy link
Member

LGTM. But merge this for after the pending release imo

@pukkandan pukkandan removed their assignment Dec 30, 2023
@pukkandan
Copy link
Member

pukkandan commented Dec 30, 2023

These days it should be safe to remove.

This is a very small change which nobody probably uses and so it's fine to remove.

But in general, I disagree. yt-dlp development moves very fast, but doesnt mean consumers of our API does the same. It's been only 3 months since deprecation, which is a tiny time window for more slow moving projects.

@pukkandan
Copy link
Member

Shouldn't

compat_HTTPError = urllib.error.HTTPError
also point to the new HTTPError?

@coletdjnz
Copy link
Member Author

These days it should be safe to remove.

This is a very small change which nobody probably uses and so it's fine to remove.

But in general, I disagree. yt-dlp development moves very fast, but doesnt mean consumers of our API does the same. It's been only 3 months since deprecation, which is a tiny time window for more slow moving projects.

Yep, I agree too. Just in this particular case it should be fine as its unlikely to be used externally.

@pukkandan
Copy link
Member

Missed

compat_urllib_HTTPError = urllib.error.HTTPError

@coletdjnz
Copy link
Member Author

Missed

compat_urllib_HTTPError = urllib.error.HTTPError

that says explicitly urllib though?

@pukkandan
Copy link
Member

They are supposed to be the same thing and was added due to ytdl-org/youtube-dl@249f2b6

@coletdjnz coletdjnz merged commit 811d298 into yt-dlp:master Jan 20, 2024
15 checks passed
@coletdjnz coletdjnz deleted the networking/remove-compat-http-error branch January 20, 2024 02:26
aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
Use `yt_dlp.networking.exceptions.HTTPError`.
`_CompatHTTPError` was to help with transition to the networking framework.

Authored by: coletdjnz
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
networking core networking related
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants