Skip to content

Conversation

Mantisus
Copy link
Collaborator

Description

  • Add retire_browser_after_page_count parameter for BrowserPool

@Mantisus
Copy link
Collaborator Author

Parameter added following the Crawlee TS.

@Mantisus Mantisus requested review from Copilot, Pijukatel and vdusek June 24, 2025 03:26
@Mantisus Mantisus self-assigned this Jun 24, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a new retire_browser_after_page_count option to retire browsers after they’ve opened a certain number of pages.

  • Tracks and exposes a per-browser total_pages count
  • Introduces retire_browser_after_page_count setting and retirement logic in BrowserPool
  • Adds a unit test for the new retirement behavior

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tests/unit/browsers/test_browser_pool.py New parametrized test for retirement behavior
src/crawlee/browsers/_playwright_browser_controller.py Add total_pages state, property, and increment
src/crawlee/browsers/_browser_pool.py Add constructor parameter, doc updates, and retire logic
src/crawlee/browsers/_browser_controller.py Declare abstract total_pages property in interface
Comments suppressed due to low confidence (2)

src/crawlee/browsers/_browser_pool.py:68

  • The docstring for identify_inactive_browsers_interval mistakenly says 'retired'; it should read 'inactive' to accurately reflect that this interval only flags browsers as inactive.
                as retired.

src/crawlee/browsers/_browser_controller.py:31

  • Since total_pages is now abstract on BrowserController, ensure every concrete implementation (not just Playwright) overrides this property to prevent instantiation errors or missing behavior.
    def total_pages(self) -> int:

Copy link
Collaborator

@janbuchar janbuchar left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor issues

@Mantisus Mantisus requested a review from Pijukatel June 25, 2025 12:24
@Pijukatel Pijukatel merged commit 603aa2b into apify:master Jun 26, 2025
43 of 44 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.

3 participants