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

Bugfix: songs being dropped when adding them to the queue #56

Merged
merged 7 commits into from
Nov 28, 2021

Conversation

michael-je
Copy link
Owner

@michael-je michael-je commented Nov 23, 2021

This PR fixes a bug where songs are often dropped when trying to add them to the queue. It does this by fixing the "dislike_count" error that popped up after youtube removed dislikes on their videos.

A side effect is that it seems to speed up the time it takes to add multiple songs to the queue, so it might help with #49 (comment).

I've simply customized pafy.new() so that the _dislikes variable of its youtube backend has a default value if none is found.

I'll probably add a PR to the pafy repository as well Someone beat me to it: mps-youtube/pafy#305. In the meantime though, this dirty fix should do the trick.

@michael-je michael-je requested review from gardarandri, morgaesis and thorduragust and removed request for gardarandri and morgaesis November 23, 2021 14:40
@michael-je michael-je changed the title Customize Pafy so that it ignores "dislike_count" errors Fix songs being dropped when adding them to the queue Nov 23, 2021
@michael-je michael-je changed the title Fix songs being dropped when adding them to the queue Bugfix: songs being dropped when adding them to the queue Nov 23, 2021
@michael-je michael-je added the bug Something isn't working label Nov 24, 2021
@morgaesis
Copy link
Collaborator

I don't like copying pafy source (with modifications) to fix something which should be solved upstream. We should rather pressure the upstream pafy repo to fix this and publish the update.

Our bot isn't critical, nor in widespread usage, so I don't this PR is warranted, at least not like this. If we had many users and some promise of uptime, that'd be a different story.

@michael-je
Copy link
Owner Author

michael-je commented Nov 24, 2021

What's the issue?

I've already done what I can to pressure this upstream by approving the PR I linked.
I would like the bot to be functional so that we can use it.
These changes can be removed at any moment, and without breaking anything, by changing a single line in the main script.
The source code is under the LGPLv3 license, so no issue there.
Also, the last update release on the pafy repository is from 2 years ago, so we really have no idea when this will be resolved upstream.

And most importantly...it works :o

@morgaesis
Copy link
Collaborator

morgaesis commented Nov 25, 2021

All right. I thought the pafy repo were actively maintained. Disregard my last comment.

If we copy the source from a GPL repo, we might need to set the licence for this repo to GPL as well. Currently there is no licence.

Copy link
Collaborator

@morgaesis morgaesis left a comment

Choose a reason for hiding this comment

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

LGTM 👍

  • Some ideas for handling lack of *like_count.
  • Please add a licence to this repo. I don't know how to lawyer; can the licence be MIT? Apache? GPL? I think GPLv3 is a safe choice, since the code you're "borrowing" is GPLv3 as well.

pafy_fixed/backend_youtube_dl_fixed.py Show resolved Hide resolved
pafy_fixed/backend_youtube_dl_fixed.py Outdated Show resolved Hide resolved
@michael-je
Copy link
Owner Author

Fixed :) thanks for reminding me about the license!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants