Fix/hardcoded concurrency

[kaya70875]

No description provided.

[what-the-diff]

PR Summary

• Introduction of new Feature - Control Transcript Request Concurrency

  • This update introduces a new feature that allows you to control the number of simultaneous transcript requests. It adds a parameter termed max_concurrent_requests which can be used to limit the concurrency of these requests thereby giving you more control over the fetching processes.

• Updates to Documentation

  • Detailed guidelines and usage examples for this new feature have been added to the documentation. This includes the README, CLI documentation and also within the context of fetching transcripts.

• Inclusion in Code Implementation

  • The new parameter max_concurrent_requests has been integrated into the argument set for the fetching processes, passed to the transcript fetcher during initialization, and added as an attribute to the FetchOptions class in fetch_config.py.

• Release Note and Changelog Updates

  • The addition of this new parameter is mentioned in the release notes and changelog. This allows users to quickly understand the highlights of the new release.

Note: The default value for max_concurrent_requests has been set to 20. However, this can be overwritten based on the user's needs.

[qodo-code-review]

PR Summary by Qodo

Make transcript fetch concurrency configurable via FetchOptions and CLI
✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

Description

• Add configurable transcript request concurrency via FetchOptions.max_concurrent_requests.
• Expose concurrency control in CLI as --max-concurrency (default: 20).
• Document the new option across README, CLI docs, and release notes.

Diagram

graph TD
  A["CLI (--max-concurrency)"] --> B["FetchOptions.max_concurrent_requests"] --> C["YTFetcher _core"] --> D["TranscriptFetcher"] --> E(["ThreadPoolExecutor"]) --> F(["requests HTTPAdapter pool"]) 
  D -. "sets pool size" .-> F

Loading

High-Level Assessment

The approach is appropriate: a single concurrency knob is threaded from CLI/config into the only place that enforces parallelism (TranscriptFetcher), and is consistently applied to both the executor and the requests connection pool. Considered alternatives (separate knobs for executor vs HTTP pool, or auto-tuning based on proxy pool size) add complexity without clear benefit for this library’s current scope.

Files changed (9) +95 / -7

Enhancement (4) +17 / -6

_cli.pyAdd '--max-concurrency' flag and wire into FetchOptions +3/-1

Add '--max-concurrency' flag and wire into FetchOptions

• Adds a new CLI argument '--max-concurrency' (default 20) under Network Options. Passes the parsed value into 'FetchOptions(max_concurrent_requests=...)' when constructing the fetcher options.

ytfetcher/_cli.py

_core.pyPropagate max concurrency from FetchOptions into TranscriptFetcher +2/-1

Propagate max concurrency from FetchOptions into TranscriptFetcher

• Updates transcript fetcher construction to forward 'options.max_concurrent_requests' into 'TranscriptFetcher(...)' so the concurrency limit is applied during transcript retrieval.

ytfetcher/_core.py

_transcript_fetcher.pyReplace hardcoded worker count with configurable max concurrency +9/-4

Replace hardcoded worker count with configurable max concurrency

• Adds a 'max_concurrent_requests' constructor parameter (default 20) and stores it on the instance. Uses the value for both 'ThreadPoolExecutor(max_workers=...)' and the requests 'HTTPAdapter' connection pool sizing.

ytfetcher/_transcript_fetcher.py

fetch_config.pyAdd 'max_concurrent_requests' to FetchOptions dataclass +3/-0

Add 'max_concurrent_requests' to FetchOptions dataclass

• Introduces 'FetchOptions.max_concurrent_requests' with a default of 20 and documentation describing it as the maximum number of concurrent transcript network requests.

ytfetcher/config/fetch_config.py

Documentation (5) +78 / -1

CHANGELOG.mdDocument new concurrency option and tidy fixed entries +4/-1

Document new concurrency option and tidy fixed entries

• Adds a changelog entry for 'FetchOptions.max_concurrent_requests' and the '--max-concurrency' CLI option. Rewords/normalizes items under the Fixed section.

CHANGELOG.md

README.mdAdd README section describing transcript concurrency controls +37/-0

Add README section describing transcript concurrency controls

• Documents the new 'max_concurrent_requests' option in the FetchOptions list and adds a dedicated section with Python and CLI usage examples. Also adds a CLI subsection explaining '--max-concurrency' and its default.

README.md

cli.mdDocument '--max-concurrency' and add usage example +17/-0

Document '--max-concurrency' and add usage example

• Introduces a Network Options subsection describing '--max-concurrency', its default (20), and when to adjust it. Adds an example showing reduced concurrency during larger fetches.

docs/cli.md

index.mdAdd concurrency documentation to main docs page +19/-0

Add concurrency documentation to main docs page

• Adds a new section explaining default transcript concurrency and how to set 'max_concurrent_requests' via FetchOptions, with a code example.

docs/index.md

release-notes.mdAdd release note entry for configurable transcript concurrency +1/-0

Add release note entry for configurable transcript concurrency

• Notes the addition of 'FetchOptions.max_concurrent_requests' and the corresponding '--max-concurrency' CLI flag.

docs/release-notes.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Concurrency not validated ✓ Resolved 🐞 Bug ☼ Reliability

Description

max_concurrent_requests is accepted without validation and used directly for
ThreadPoolExecutor(max_workers=...) and HTTPAdapter pool sizing, so values <= 0 will crash
transcript fetching at runtime. Very large values can also spawn excessive threads/connections and
degrade stability.

Code

ytfetcher/_transcript_fetcher.py[149]

+            with futures.ThreadPoolExecutor(max_workers=self.max_concurrent_requests) as executor:

Evidence

The CLI exposes --max-concurrency as an unconstrained integer and passes it into
FetchOptions.max_concurrent_requests, which is then forwarded into TranscriptFetcher and used
directly for both HTTPAdapter pool sizing and ThreadPoolExecutor sizing; there is no guard against
values <= 0.

ytfetcher/_cli.py[81-93]
ytfetcher/_cli.py[250-284]
ytfetcher/_core.py[259-267]
ytfetcher/_transcript_fetcher.py[100-121]
ytfetcher/_transcript_fetcher.py[145-153]
ytfetcher/config/fetch_config.py[20-58]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A user-controlled `max_concurrent_requests` value is passed from the CLI/API all the way into `TranscriptFetcher` and is used directly to size both the requests connection pool and the thread pool. Values like `0` or negative numbers will cause runtime failures during transcript fetching.

## Issue Context
- CLI defines `--max-concurrency` as a plain `int` and passes it through `FetchOptions` without bounds.
- `TranscriptFetcher` then uses that value for `HTTPAdapter(pool_connections=..., pool_maxsize=...)` and `ThreadPoolExecutor(max_workers=...)`.

## Fix Focus Areas
- ytfetcher/_cli.py[81-93]
- ytfetcher/_cli.py[250-294]
- ytfetcher/config/fetch_config.py[20-58]
- ytfetcher/_core.py[259-267]
- ytfetcher/_transcript_fetcher.py[77-121]
- ytfetcher/_transcript_fetcher.py[145-153]

## Implementation notes
- Add validation in at least one central place (preferably `TranscriptFetcher.__init__` and/or `FetchOptions.__post_init__`) to enforce `max_concurrent_requests >= 1`.
- Also consider adding CLI-side validation (custom argparse type) so users get an immediate, friendly error message.
- Optionally clamp to a reasonable upper bound (or document that large values may be harmful) to avoid accidental resource exhaustion.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---