Skip to content

feat(yts): Add youtube cookies support#117

Merged
Rushaway merged 6 commits intosrcdslab:masterfrom
Dolly132:main-dolly0
Mar 26, 2026
Merged

feat(yts): Add youtube cookies support#117
Rushaway merged 6 commits intosrcdslab:masterfrom
Dolly132:main-dolly0

Conversation

@Dolly132
Copy link
Contributor

@Dolly132 Dolly132 commented Mar 26, 2026

Solves #68

@Rushaway
Copy link
Member

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

  1. 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:

    if cookies and not os.path.isfile(cookies):
        self.logger.warning(f"Cookies file not found: {cookies}, ignoring")
        cookies = ""
  2. 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.

  3. 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

  4. 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!

@Rushaway Rushaway merged commit 99228d6 into srcdslab:master Mar 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants