ronantakizawa/codereview-bench huggingface dataset:
Repository: apify/crawlee-python
File: docs/examples/code/playwright_block_requests.py (Python)

Code before:
import asyncio

from crawlee.crawlers import PlaywrightCrawler, PlaywrightCrawlingContext, PlaywrightPreNavCrawlingContext


async def main() -> None:
    crawler = PlaywrightCrawler(
        # Limit the crawl to max requests. Remove or increase it for crawling all links.
        max_requests_per_crawl=10,
    )

    # Define the default request handler, which will be called for every request.
    @crawler.router.default_handler
    async def request_handler(context: PlaywrightCrawlingContext) -> None:
        context.log.info(f'Processing {context.request.url} ...')

        await context.enqueue_links()

    # Define the hook, which will be called before every request.
    @crawler.pre_navigation_hook
    async def navigation_hook(context: PlaywrightPreNavCrawlingContext) -> None:
        context.log.info(f'Navigating to {context.request.url} ...')

        # block all requests to URLs that include `adsbygoogle.js` and also all defaults.
        await context.block_requests(extra_url_patterns=['adsbygoogle.js'])

    # Run the crawler with the initial list of URLs.
    await crawler.run(['https://crawlee.dev/'])


if __name__ == '__main__':
    asyncio.run(main())


Review comment (suggestion):
```suggestion
        # Block all requests to URLs that include `adsbygoogle.js` and also all defaults.
```

Code after:
import asyncio

from crawlee.crawlers import PlaywrightCrawler, PlaywrightCrawlingContext, PlaywrightPreNavCrawlingContext


async def main() -> None:
    crawler = PlaywrightCrawler(
        # Limit the crawl to max requests. Remove or increase it for crawling all links.
        max_requests_per_crawl=10,
    )

    # Define the default request handler, which will be called for every request.
    @crawler.router.default_handler
    async def request_handler(context: PlaywrightCrawlingContext) -> None:
        context.log.info(f'Processing {context.request.url} ...')

        await context.enqueue_links()

    # Define the hook, which will be called before every request.
    @crawler.pre_navigation_hook
    async def navigation_hook(context: PlaywrightPreNavCrawlingContext) -> None:
        context.log.info(f'Navigating to {context.request.url} ...')

        # Block all requests to URLs that include `adsbygoogle.js` and also all defaults.
        await context.block_requests(extra_url_patterns=['adsbygoogle.js'])

    # Run the crawler with the initial list of URLs.
    await crawler.run(['https://crawlee.dev/'])


if __name__ == '__main__':
    asyncio.run(main())
Repository: apify/crawlee-python
File: src/crawlee/crawlers/_playwright/_types.py (Python)

Code before:
from __future__ import annotations

from typing import Protocol

from crawlee._utils.docs import docs_group


@docs_group('Functions')
class BlockRequestsFunction(Protocol):
    """A function for blocking unwanted HTTP requests during page loads in PlaywrightCrawler.

    It simplifies the process of blocking specific HTTP requests during page navigation.
    The function allows blocking both default resource types (like images, fonts, stylesheets) and custom URL patterns.

    Default patterns for blocking ['.css', '.webp', '.jpg', '.jpeg', '.png', '.svg', '.gif', '.woff', '.pdf', '.zip']
    """

    async def __call__(
        self, url_patterns: list[str] | None = None, extra_url_patterns: list[str] | None = None
    ) -> None:
        """Call dunder method.

        Args:
            url_patterns: List of URL patterns to block. If None, uses default patterns.
            extra_url_patterns: Additional URL patterns to append to the main patterns list.
        """


Review comment (suggestion):
Oh,  this is risky as it could become easily outdated if we update the value of `_DEFAULT_BLOCK_REQUEST_URL_PATTERNS`. I would prefer not to mention it here, or alternatively, we could consider inlining the variable's value directly into the docstring, if it is possible.

Code after:
from __future__ import annotations

from typing import Protocol

from crawlee._utils.docs import docs_group


@docs_group('Functions')
class BlockRequestsFunction(Protocol):
    """A function for blocking unwanted HTTP requests during page loads in PlaywrightCrawler.

    It simplifies the process of blocking specific HTTP requests during page navigation.
    The function allows blocking both default resource types (like images, fonts, stylesheets) and custom URL patterns.
    """

    async def __call__(
        self, url_patterns: list[str] | None = None, extra_url_patterns: list[str] | None = None
    ) -> None:
        """Call dunder method.

        Args:
            url_patterns: List of URL patterns to block. If None, uses default patterns.
            extra_url_patterns: Additional URL patterns to append to the main patterns list.
        """
Repository: apify/crawlee-python
File: docs/introduction/code/03_transform_request.py (Python)

Code before:
from __future__ import annotations

from crawlee import HttpHeaders, RequestOptions
from crawlee.crawlers import BeautifulSoupCrawler, BeautifulSoupCrawlingContext


def transform_request(request: RequestOptions) -> RequestOptions | None:
    # Skip requests to PDF files
    if request['url'].endswith('.pdf'):
        return None

    # Add custom headers to requests to specific sections
    if '/docs' in request['url']:
        request['headers'] = HttpHeaders({'Custom-Header': 'value'})

    # Add label for certain URLs
    if '/blog' in request['url']:
        request['label'] = 'BLOG'

    return request


async def main() -> None:
    crawler = BeautifulSoupCrawler(max_requests_per_crawl=50)

    @crawler.router.default_handler
    async def request_handler(context: BeautifulSoupCrawlingContext) -> None:
        context.log.info(f'Processing {context.request.url}.')

        # Transfor request befor enqueue
        await context.enqueue_links(transform_request_function=transform_request)


Review comment (suggestion):
Maybe do not call the variable request. I know there is a type hint next to it. But my first though was that I should actually pass the request object to it.

Code after:
from __future__ import annotations

from crawlee import HttpHeaders, RequestOptions, RequestTransformAction
from crawlee.crawlers import BeautifulSoupCrawler, BeautifulSoupCrawlingContext


def transform_request(request_options: RequestOptions) -> RequestOptions | RequestTransformAction:
    # Skip requests to PDF files
    if request_options['url'].endswith('.pdf'):
        return 'skip'

    if '/docs' in request_options['url']:
        # Add custom headers to requests to specific URLs
        request_options['headers'] = HttpHeaders({'Custom-Header': 'value'})

    elif '/blog' in request_options['url']:
        # Add label for certain URLs
        request_options['label'] = 'BLOG'

    else:
        # Signal that the request should proceed without any transformation
        return 'unchanged'

    return request_options


async def main() -> None:
    crawler = BeautifulSoupCrawler(max_requests_per_crawl=50)

    @crawler.router.default_handler
    async def request_handler(context: BeautifulSoupCrawlingContext) -> None:
        context.log.info(f'Processing {context.request.url}.')
Repository: apify/crawlee-python
File: docs/introduction/code/03_transform_request.py (Python)

Code before:
    if request_options['url'].endswith('.pdf'):
        return 'skip'

    if '/docs' in request_options['url']:
        # Add custom headers to requests to specific URLs
        request_options['headers'] = HttpHeaders({'Custom-Header': 'value'})

    elif '/blog' in request_options['url']:
        # Add label for certain URLs
        request_options['label'] = 'BLOG'

    else:
        # Signal that the request should proceed without any transformation
        return 'unchanged'

    return request_options


async def main() -> None:
    crawler = BeautifulSoupCrawler(max_requests_per_crawl=50)

    @crawler.router.default_handler
    async def request_handler(context: BeautifulSoupCrawlingContext) -> None:
        context.log.info(f'Processing {context.request.url}.')

        # Transfor request befor enqueue
        await context.enqueue_links(transform_request_function=transform_request)

    @crawler.router.handler('BLOG')
    async def blog_handler(context: BeautifulSoupCrawlingContext) -> None:
        context.log.info(f'Blog Processing {context.request.url}.')

    await crawler.run(['https://crawlee.dev/'])


Review comment (suggestion):
```suggestion
        # Transform request before enqueueing
```

Code after:
    if request_options['url'].endswith('.pdf'):
        return 'skip'

    if '/docs' in request_options['url']:
        # Add custom headers to requests to specific URLs
        request_options['headers'] = HttpHeaders({'Custom-Header': 'value'})

    elif '/blog' in request_options['url']:
        # Add label for certain URLs
        request_options['label'] = 'BLOG'

    else:
        # Signal that the request should proceed without any transformation
        return 'unchanged'

    return request_options


async def main() -> None:
    crawler = BeautifulSoupCrawler(max_requests_per_crawl=50)

    @crawler.router.default_handler
    async def request_handler(context: BeautifulSoupCrawlingContext) -> None:
        context.log.info(f'Processing {context.request.url}.')

        # Transform request before enqueueing
        await context.enqueue_links(transform_request_function=transform_request)

    @crawler.router.handler('BLOG')
    async def blog_handler(context: BeautifulSoupCrawlingContext) -> None:
        context.log.info(f'Blog Processing {context.request.url}.')

    await crawler.run(['https://crawlee.dev/'])
Repository: apify/crawlee-python
File: src/crawlee/http_clients/_curl_impersonate.py (Python)

Code before:
            kwargs: dict[str, Any] = {
                'proxy': proxy_url,
                'impersonate': CURL_DEFAULT_CHROME,
            }

            # Update the default kwargs with any additional user-provided kwargs.
            kwargs.update(self._async_session_kwargs)

            # Create and store the new session with the specified kwargs.
            self._client_by_proxy_url[proxy_url] = AsyncSession(**kwargs)
            self._client_by_proxy_url[proxy_url].cookies = _EmptyCookies()

        return self._client_by_proxy_url[proxy_url]

    @staticmethod
    def _is_proxy_error(error: CurlRequestError) -> bool:
        """Helper to check whether the given error is a proxy-related error."""
        if any(needle in str(error) for needle in ROTATE_PROXY_ERRORS):
            return True

        if isinstance(error, CurlProxyError):  # noqa: SIM103
            return True

        return False

    def _get_cookies(self, curl: Curl) -> dict[str, str]:
        cookies = {}
        for curl_cookie in curl.getinfo(CurlInfo.COOKIELIST):  # type: ignore[union-attr]
            curl_morsel = CurlMorsel.from_curl_format(curl_cookie)  # type: ignore[arg-type]
            cookies[curl_morsel.name] = curl_morsel.value
        return cookies


Review comment (suggestion):
```suggestion
    @staticmethod
    def _get_cookies(self, curl: Curl) -> dict[str, str]:
```

Code after:
            kwargs: dict[str, Any] = {
                'proxy': proxy_url,
                'impersonate': CURL_DEFAULT_CHROME,
            }

            # Update the default kwargs with any additional user-provided kwargs.
            kwargs.update(self._async_session_kwargs)

            # Create and store the new session with the specified kwargs.
            self._client_by_proxy_url[proxy_url] = AsyncSession(**kwargs)
            self._client_by_proxy_url[proxy_url].cookies = _EmptyCookies()

        return self._client_by_proxy_url[proxy_url]

    @staticmethod
    def _is_proxy_error(error: CurlRequestError) -> bool:
        """Helper to check whether the given error is a proxy-related error."""
        if any(needle in str(error) for needle in ROTATE_PROXY_ERRORS):
            return True

        if isinstance(error, CurlProxyError):  # noqa: SIM103
            return True

        return False

    @staticmethod
    def _get_cookies(curl: Curl) -> dict[str, str]:
        cookies = {}
        for curl_cookie in curl.getinfo(CurlInfo.COOKIELIST):  # type: ignore[union-attr]
            curl_morsel = CurlMorsel.from_curl_format(curl_cookie)  # type: ignore[arg-type]
            cookies[curl_morsel.name] = curl_morsel.value
        return cookies
Repository: apify/crawlee-python
File: tests/unit/crawlers/_http/test_http_crawler.py (Python)

Code before:
        # The session with the cookie is retire. The next request should be made by a session without a cookie
        if context.request.unique_key == '1':
            await context.add_requests(
                [
                    Request.from_url(str(httpbin.with_path('/cookies')), unique_key='2', label='cookies'),
                ]
            )

    await crawler.run(
        [
            str(httpbin.with_path('/cookies/set').extend_query(a=1)),
        ]
    )

    assert len(sessions_cookies) == 2
    assert len(response_cookies) == 2

    cookie_session_id = sessions_id[0]
    clean_session_id = sessions_id[1]

    assert cookie_session_id != clean_session_id

    # The initiated cookies must match in both the response and the session store
    assert sessions_cookies[cookie_session_id] == response_cookies[cookie_session_id] == {'a': '1'}

    # For a clean session, the cookie should not be in the sesstion store or in the response
    # This way we can be sure that no cookies are being leaked through the http client
    assert sessions_cookies[clean_session_id] == response_cookies[clean_session_id] == {}


Review comment (suggestion):
```suggestion
    # For a clean session, the cookie should not be in the session store or in the response
```

Code after:
    await crawler.run(
        [
            # The first request sets the cookie in the session
            str(httpbin.with_path('/cookies/set').extend_query(a=1)),
            # With the second request, we check the cookies in the session and set retire
            Request.from_url(str(httpbin.with_path('/cookies')), unique_key='1', user_data={'retire_session': True}),
            # The third request is made with a new session to make sure it does not use another session's cookies
            Request.from_url(str(httpbin.with_path('/cookies')), unique_key='2'),
        ]
    )

    assert len(sessions_cookies) == 2
    assert len(response_cookies) == 2

    assert sessions_ids[0] == sessions_ids[1]

    cookie_session_id = sessions_ids[0]
    clean_session_id = sessions_ids[2]

    assert cookie_session_id != clean_session_id

    # The initiated cookies must match in both the response and the session store
    assert sessions_cookies[cookie_session_id] == response_cookies[cookie_session_id] == {'a': '1'}

    # For a clean session, the cookie should not be in the session store or in the response
    # This way we can be sure that no cookies are being leaked through the http client
    assert sessions_cookies[clean_session_id] == response_cookies[clean_session_id] == {}
Repository: apify/crawlee-python
File: src/crawlee/browsers/_playwright_browser.py (Python)

Code before:
from __future__ import annotations

import shutil
import tempfile
from functools import partial
from logging import getLogger
from typing import TYPE_CHECKING, Any

if TYPE_CHECKING:
    from pathlib import Path

    from playwright.async_api import BrowserContext, BrowserType

logger = getLogger(__name__)


class PlaywrightPersistentBrowser:
    """Wrapper for Browser that uses persistent context under the hood."""

    def __init__(
        self,
        browser_type: BrowserType,
        user_data_dir: str | Path | None,
        browser_launch_options: dict[str, Any],
    ) -> None:
        self._browser_type = browser_type
        self._browser_launch_options = browser_launch_options
        self._user_data_dir = user_data_dir

        self._context: BrowserContext | None = None
        self._is_connected = True

    @property
    def browser_type(self) -> BrowserType:
        return self._browser_type

    def is_connected(self) -> bool:
        return self._is_connected

    async def new_context(self, **context_options: Any) -> BrowserContext:
        """Creates persistent context instead of regular one. Merges launch options with context options."""
        if self._context:
            raise RuntimeError('Persistent browser can have only one context')

Review comment (suggestion):
```suggestion
    """Wrapper for browser that uses persistent context under the hood."""
```

Code after:
from __future__ import annotations

import asyncio
import shutil
import tempfile
from logging import getLogger
from typing import TYPE_CHECKING, Any

from playwright.async_api import Browser
from typing_extensions import override

if TYPE_CHECKING:
    from pathlib import Path

    from playwright.async_api import BrowserContext, BrowserType, CDPSession, Page

logger = getLogger(__name__)


class PlaywrightPersistentBrowser(Browser):
    """A wrapper for Playwright's `Browser` that operates with a persistent context.

    It utilizes Playwright's persistent browser context feature, maintaining user data across sessions.
    While it follows the same interface as Playwright's `Browser` class, there is no abstract base class
    enforcing this. There is a limitation that only a single persistent context is allowed.
    """

    _TMP_DIR_PREFIX = 'apify-playwright-firefox-taac-'

    def __init__(
        self,
        browser_type: BrowserType,
        user_data_dir: str | Path | None,
        browser_launch_options: dict[str, Any],
    ) -> None:
        self._browser_type = browser_type
        self._browser_launch_options = browser_launch_options
        self._user_data_dir = user_data_dir
        self._temp_dir: str | None = None

        self._context: BrowserContext | None = None
        self._is_connected = True
Repository: apify/crawlee-python
File: src/crawlee/_cli.py (Python)

Code before:
# ruff: noqa: TRY301, FBT002, UP007
from __future__ import annotations

import importlib.resources
import json
from pathlib import Path
from typing import Annotated, Optional, cast

try:
    import inquirer  # type: ignore[import-untyped]
    import typer
    from cookiecutter.main import cookiecutter  # type: ignore[import-untyped]
    from inquirer.render.console import ConsoleRender  # type: ignore[import-untyped]
    from rich.progress import Progress, SpinnerColumn, TextColumn
except ModuleNotFoundError as exc:
    raise ImportError("Looks like you're running just 'crawlee', try using 'crawlee[cli]'") from exc

cli = typer.Typer(no_args_is_help=True)

template_directory = importlib.resources.files('crawlee') / 'project_template'
cookiecutter_json = json.load((template_directory / 'cookiecutter.json').open())

crawler_choices = cookiecutter_json['crawler_type']
http_client_choices = cookiecutter_json['http_client']
package_manager_choices = cookiecutter_json['package_manager']
default_start_url = cookiecutter_json['start_url']


@cli.callback(invoke_without_command=True)
def callback(
    version: Annotated[
        bool,
        typer.Option(
            '-V',
            '--version',
            is_flag=True,
            help='Print Crawlee version',
        ),
    ] = False,
) -> None:
    """Crawlee is a web scraping and browser automation library."""

Review comment (suggestion):
Could we use a bit more user-friendly message? Smt like this:

> Missing required dependencies for the Crawlee CLI. It looks like you're running 'crawlee' without the CLI extra. Try using 'crawlee[cli]' instead.

Code after:
# ruff: noqa: TRY301, FBT002, UP007
from __future__ import annotations

import importlib.resources
import json
from pathlib import Path
from typing import Annotated, Optional, cast

try:
    import inquirer  # type: ignore[import-untyped]
    import typer
    from cookiecutter.main import cookiecutter  # type: ignore[import-untyped]
    from inquirer.render.console import ConsoleRender  # type: ignore[import-untyped]
    from rich.progress import Progress, SpinnerColumn, TextColumn
except ModuleNotFoundError as exc:
    raise ImportError(
        "Missing required dependencies for the Crawlee CLI. It looks like you're running 'crawlee' "
        "without the CLI extra. Try using 'crawlee[cli]' instead."
    ) from exc

cli = typer.Typer(no_args_is_help=True)

template_directory = importlib.resources.files('crawlee') / 'project_template'
cookiecutter_json = json.load((template_directory / 'cookiecutter.json').open())

crawler_choices = cookiecutter_json['crawler_type']
http_client_choices = cookiecutter_json['http_client']
package_manager_choices = cookiecutter_json['package_manager']
default_start_url = cookiecutter_json['start_url']


@cli.callback(invoke_without_command=True)
def callback(
    version: Annotated[
        bool,
        typer.Option(
            '-V',
            '--version',
            is_flag=True,
            help='Print Crawlee version',
        ),
Repository: apify/crawlee-python
File: tests/unit/_utils/test_console.py (Python)

Code before:
from __future__ import annotations

from crawlee._utils.console import make_table


def test_empty_input() -> None:
    assert make_table([]) == ''


def test_empty_row() -> None:
    assert make_table([()]) == ''


def test_single_column() -> None:
    result = make_table([('test',)])
    lines = result.split('\n')
    assert len(lines) == 3
    assert lines[1] == '│ test │'


def test_two_columns() -> None:
    data = [('Name', 'Age'), ('Alice', '30'), ('Bob', '25')]
    result = make_table(data)
    lines = result.split('\n')
    assert lines == ['┌───────┬─────┐', '│ Name  │ Age │', '│ Alice │ 30  │', '│ Bob   │ 25  │', '└───────┴─────┘']


def test_long_content_truncation() -> None:
    data = [('Short', 'VeryVeryVeryLongContent')]
    result = make_table(data, width=25)
    lines = result.split('\n')
    assert lines == ['┌───────────┬───────────┐', '│ Short     │ VeryVe... │', '└───────────┴───────────┘']


Review comment (suggestion):
(Just a suggestion)
The test are good, but not so visually obvious. I tried if the ruff would allow to show nicely formatted multi-line text without breaking it and it can still be done

For example this shows the :

```
    data = [('Name', 'Age'), ('Alice', '30'), ('Bob', '25')]
    assert (
        make_table(data)
        == """\
┌───────┬─────┐
│ Name  │ Age │
│ Alice │ 30  │
│ Bob   │ 25  │
└───────┴─────┘"""
    )
```




Code after:
from __future__ import annotations

from crawlee._utils.console import make_table


def test_empty_input() -> None:
    assert make_table([]) == ''


def test_empty_row() -> None:
    assert make_table([()]) == ''


def test_single_column() -> None:
    result = make_table([('test',)])
    lines = result.split('\n')
    assert len(lines) == 3
    assert lines[1] == '│ test │'


def test_two_columns() -> None:
    data = [('Name', 'Age'), ('Alice', '30'), ('Bob', '25')]
    result = make_table(data)
    lines = result.split('\n')
    # fmt: off
    assert lines == ['┌───────┬─────┐',
                     '│ Name  │ Age │',
                     '│ Alice │ 30  │',
                     '│ Bob   │ 25  │',
                     '└───────┴─────┘']
    # fmt: on


def test_long_content_truncation() -> None:
    data = [('Short', 'VeryVeryVeryLongContent')]
    result = make_table(data, width=25)
    lines = result.split('\n')
    # fmt: off
    assert lines == ['┌───────────┬───────────┐',
                     '│ Short     │ VeryVe... │',
                     '└───────────┴───────────┘']
    # fmt: on