You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hey @Dolly132, thanks for the PR! Small, focused, and does exactly one thing — that's the right approach. Here are my thoughts:
✅ What's good
Great scope: 3 files, ~25 lines, single clear objective
Consistent propagation of the cookies param throughout the whole call chain (get_url_youtube_info, get_first_valid_entry, and all call sites in Commands.py)
The os.path.isfile(cookies) guard is a nice touch — avoids cryptic yt-dlp errors if the file doesn't exist
Opt-in by default ("" default value), so it's fully backward-compatible
⚠️ A few things worth addressing
Silent failure when cookies file is missing — if cookies is set in the config but the file doesn't exist, it gets reset to "" with no feedback. A warning log would really help with debugging:
ifcookiesandnotos.path.isfile(cookies):
self.logger.warning(f"Cookies file not found: {cookies}, ignoring")
cookies=""
The default path in config.json — having "config/cookies.txt" as the example value is a bit dangerous: someone might copy the config as-is, the file won't exist, and cookies will be silently ignored (see point above). Maybe use an empty string "" as the default instead, or add a comment.
Check if the file is not empty - having "config/cookies.txt" is correct check, but the fail be empty and can lead to a silent failure. A warning log would really help with debugging
No documentation — the cookies file format (Netscape HTTP Cookie File) is not obvious for end users. A short note in the README about how to generate it (e.g. via a browser extension like "Get cookies.txt") would go a long way.
Overall this is solid and mergeable. The silent failure on missing file is the one thing I'd fix before merging — the rest are nice-to-haves. Good work!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Solves #68