ronantakizawa/codereview-bench huggingface dataset:
Repository: apify/crawlee-python
File: tests/unit/storages/test_key_value_store.py (Python)

Code before:
    keys = [item.key async for item in key_value_store.iterate_keys()]
    assert len(keys) == 0

    for i in range(2001):
        await key_value_store.set_value(str(i).zfill(4), i)
    index = 0
    async for item in key_value_store.iterate_keys():
        assert item.key == str(index).zfill(4)
        index += 1
    assert index == 2001


async def test_static_get_set_value(key_value_store: KeyValueStore) -> None:
    await key_value_store.set_value('test-static', 'static')
    value = await key_value_store.get_value('test-static')
    assert value == 'static'


async def test_static_get_public_url(key_value_store: KeyValueStore) -> None:
    await key_value_store.set_value('test-static', 'static')
    public_url = await key_value_store.get_public_url('test-static')

    url = urlparse(public_url)
    path = url.netloc if url.netloc else url.path

    with open(path, mode='r') as f:
        content = await asyncio.to_thread(f.read)
        assert content == 'static'


Review comment (suggestion):
```suggestion
    with open(path) as f:  # noqa: ASYNC230
```

Code after:
    keys = [item.key async for item in key_value_store.iterate_keys()]
    assert len(keys) == 0

    for i in range(2001):
        await key_value_store.set_value(str(i).zfill(4), i)
    index = 0
    async for item in key_value_store.iterate_keys():
        assert item.key == str(index).zfill(4)
        index += 1
    assert index == 2001


async def test_static_get_set_value(key_value_store: KeyValueStore) -> None:
    await key_value_store.set_value('test-static', 'static')
    value = await key_value_store.get_value('test-static')
    assert value == 'static'


async def test_get_public_url_raises_for_non_existing_key(key_value_store: KeyValueStore) -> None:
    with pytest.raises(ValueError, match='was not found'):
        await key_value_store.get_public_url('i-do-not-exist')


async def test_get_public_url(key_value_store: KeyValueStore) -> None:
    await key_value_store.set_value('test-static', 'static')
    public_url = await key_value_store.get_public_url('test-static')

    url = urlparse(public_url)
    path = url.netloc if url.netloc else url.path

    with open(path) as f:  # noqa: ASYNC230
        content = await asyncio.to_thread(f.read)
        assert content == 'static'
Repository: apify/crawlee-python
File: tests/unit/storages/test_key_value_store.py (Python)

Code before:
    assert str_value == 'string'
    assert int_value == 123
    assert dict_value['abc'] == '123'
    assert non_existent_value is None


async def test_for_each_key(key_value_store: KeyValueStore) -> None:
    keys = [item.key async for item in key_value_store.iterate_keys()]
    assert len(keys) == 0

    for i in range(2001):
        await key_value_store.set_value(str(i).zfill(4), i)
    index = 0
    async for item in key_value_store.iterate_keys():
        assert item.key == str(index).zfill(4)
        index += 1
    assert index == 2001


async def test_static_get_set_value(key_value_store: KeyValueStore) -> None:
    await key_value_store.set_value('test-static', 'static')
    value = await key_value_store.get_value('test-static')
    assert value == 'static'


async def test_static_get_public_url(key_value_store: KeyValueStore) -> None:
    await key_value_store.set_value('test-static', 'static')
    public_url = await key_value_store.get_public_url('test-static')

    url = urlparse(public_url)
    path = url.netloc if url.netloc else url.path

    with open(path) as f:  # noqa: ASYNC230
        content = await asyncio.to_thread(f.read)
        assert content == 'static'


Review comment (suggestion):
```suggestion
async def test_get_public_url_raises_for_non_existing_key(key_value_store: KeyValueStore) -> None:
    with pytest.raises(ValueError, match='was not found'):
        await key_value_store.get_public_url('i-do-not-exist')


async def test_get_public_url(key_value_store: KeyValueStore) -> None:
```

Code after:
    assert str_value == 'string'
    assert int_value == 123
    assert dict_value['abc'] == '123'
    assert non_existent_value is None


async def test_for_each_key(key_value_store: KeyValueStore) -> None:
    keys = [item.key async for item in key_value_store.iterate_keys()]
    assert len(keys) == 0

    for i in range(2001):
        await key_value_store.set_value(str(i).zfill(4), i)
    index = 0
    async for item in key_value_store.iterate_keys():
        assert item.key == str(index).zfill(4)
        index += 1
    assert index == 2001


async def test_static_get_set_value(key_value_store: KeyValueStore) -> None:
    await key_value_store.set_value('test-static', 'static')
    value = await key_value_store.get_value('test-static')
    assert value == 'static'


async def test_get_public_url_raises_for_non_existing_key(key_value_store: KeyValueStore) -> None:
    with pytest.raises(ValueError, match='was not found'):
        await key_value_store.get_public_url('i-do-not-exist')


async def test_get_public_url(key_value_store: KeyValueStore) -> None:
    await key_value_store.set_value('test-static', 'static')
    public_url = await key_value_store.get_public_url('test-static')

    url = urlparse(public_url)
    path = url.netloc if url.netloc else url.path

    with open(path) as f:  # noqa: ASYNC230
        content = await asyncio.to_thread(f.read)
        assert content == 'static'
Repository: apify/crawlee-python
File: tests/unit/_utils/test_requests.py (Python)

Code before:
from __future__ import annotations

from typing import TYPE_CHECKING

import pytest

from crawlee._utils.requests import compute_unique_key, normalize_headers, normalize_url, unique_key_to_request_id

if TYPE_CHECKING:
    from crawlee._types import HttpMethod, HttpPayload


# Tests for normalize_headers
def test_normalize_headers_whitelist() -> None:
    headers = {
        'Content-Type': 'application/json',
        'Authorization': 'Bearer token',
        'Custom-Header': 'should be ignored',
        'Accept': 'text/html',
    }
    normalized = normalize_headers(headers)
    expected = {
        'content-type': 'application/json',
        'authorization': 'Bearer token',
        'accept': 'text/html',
    }
    assert normalized == expected, 'Only whitelisted headers should be included, and keys should be lowercased.'


def test_normalize_headers_strip_values() -> None:
    headers = {
        'Content-Type': ' application/json ',
        'Authorization': '  Bearer token  ',
        'Accept': 'text/html ',
    }
    normalized = normalize_headers(headers)
    expected = {
        'content-type': 'application/json',

Review comment (suggestion):
I don't find this comment useful.
```suggestion
```


Code after:
from __future__ import annotations

from typing import TYPE_CHECKING

import pytest

from crawlee._types import HttpHeaders
from crawlee._utils.requests import compute_unique_key, normalize_url, unique_key_to_request_id

if TYPE_CHECKING:
    from crawlee._types import HttpMethod, HttpPayload


def test_unique_key_to_request_id_length() -> None:
    unique_key = 'exampleKey123'
    request_id = unique_key_to_request_id(unique_key, request_id_length=15)
    assert len(request_id) == 15, 'Request ID should have the correct length.'


def test_unique_key_to_request_id_consistency() -> None:
    unique_key = 'consistentKey'
    request_id_1 = unique_key_to_request_id(unique_key)
    request_id_2 = unique_key_to_request_id(unique_key)
    assert request_id_1 == request_id_2, 'The same unique key should generate consistent request IDs.'


@pytest.mark.parametrize(
    ('unique_key', 'expected_request_id'),
    [
        ('abc', 'ungWv48BzpBQUDe'),
        ('uniqueKey', 'xiWPs083cree7mH'),
        ('', '47DEQpj8HBSaTIm'),
        ('测试中文', 'lKPdJkdvw8MXEUp'),
        ('test+/=', 'XZRQjhoG0yjfnYD'),
    ],
    ids=[
        'basic_abc',
        'keyword_uniqueKey',
Repository: apify/crawlee-python
File: tests/unit/_utils/test_requests.py (Python)

Code before:
from __future__ import annotations

from typing import TYPE_CHECKING

import pytest

from crawlee._utils.requests import compute_unique_key, normalize_headers, normalize_url, unique_key_to_request_id

if TYPE_CHECKING:
    from crawlee._types import HttpMethod, HttpPayload


# Tests for normalize_headers
def test_normalize_headers_whitelist() -> None:
    headers = {
        'Content-Type': 'application/json',
        'Authorization': 'Bearer token',
        'Custom-Header': 'should be ignored',
        'Accept': 'text/html',
    }
    normalized = normalize_headers(headers)
    expected = {
        'content-type': 'application/json',
        'authorization': 'Bearer token',
        'accept': 'text/html',
    }
    assert normalized == expected, 'Only whitelisted headers should be included, and keys should be lowercased.'


def test_normalize_headers_strip_values() -> None:
    headers = {
        'Content-Type': ' application/json ',
        'Authorization': '  Bearer token  ',
        'Accept': 'text/html ',
    }
    normalized = normalize_headers(headers)
    expected = {
        'content-type': 'application/json',
        'authorization': 'Bearer token',

Review comment (suggestion):
```suggestion
def test_normalize_headers_includes_only_whitelisted_values() -> None:
```

Code after:
from __future__ import annotations

from typing import TYPE_CHECKING

import pytest

from crawlee._types import HttpHeaders
from crawlee._utils.requests import compute_unique_key, normalize_url, unique_key_to_request_id

if TYPE_CHECKING:
    from crawlee._types import HttpMethod, HttpPayload


def test_unique_key_to_request_id_length() -> None:
    unique_key = 'exampleKey123'
    request_id = unique_key_to_request_id(unique_key, request_id_length=15)
    assert len(request_id) == 15, 'Request ID should have the correct length.'


def test_unique_key_to_request_id_consistency() -> None:
    unique_key = 'consistentKey'
    request_id_1 = unique_key_to_request_id(unique_key)
    request_id_2 = unique_key_to_request_id(unique_key)
    assert request_id_1 == request_id_2, 'The same unique key should generate consistent request IDs.'


@pytest.mark.parametrize(
    ('unique_key', 'expected_request_id'),
    [
        ('abc', 'ungWv48BzpBQUDe'),
        ('uniqueKey', 'xiWPs083cree7mH'),
        ('', '47DEQpj8HBSaTIm'),
        ('测试中文', 'lKPdJkdvw8MXEUp'),
        ('test+/=', 'XZRQjhoG0yjfnYD'),
    ],
    ids=[
        'basic_abc',
        'keyword_uniqueKey',
        'empty_string',
Repository: apify/crawlee-python
File: website/parse_types.py (Python)

Code before:

        main_type = parse_expression(ast_node.value, full_expression)
        type_argument = parse_expression(ast_node.slice, full_expression)

        main_type['typeArguments'] = type_argument if isinstance(type_argument, list) else [type_argument]
        return main_type
    
    if (current_node_type == "Constant"):
        return {
            "type": "literal",
            "value": ast_node.value
        }
    
    # If the expression is not one of the types above, we simply print the expression
    return {
        "type": "reference",
        "name": full_expression[ast_node.col_offset:ast_node.end_col_offset]
    }

typedoc_types_path = sys.argv[1]

with open(typedoc_types_path, "r") as f:
    typedoc_out = {}
    expressions = f.read()

    expressions = json.loads(expressions)

    for expression in expressions:
        try:
            if(typedoc_out.get(expression) is None):
                typedoc_out[expression] = parse_expression(ast.parse(expression).body[0].value, expression)
        except Exception as e:
            print(f"Invalid expression encountered while parsing: {expression}")
            print(f"Error: {e}")
    
    with open(f"{os.path.splitext(typedoc_types_path)[0]}-parsed.json", "w") as f:
        f.write(json.dumps(typedoc_out, indent=4))
    

Review comment (suggestion):
```suggestion
    expressions = json.load(f)
```

Code after:
            return parse_expression(ast_node.slice.dims[0], full_expression)

        main_type = parse_expression(ast_node.value, full_expression)
        type_argument = parse_expression(ast_node.slice, full_expression)

        main_type["typeArguments"] = (
            type_argument if isinstance(type_argument, list) else [type_argument]
        )
        return main_type

    if current_node_type == "Constant":
        return {"type": "literal", "value": ast_node.value}

    # If the expression is not one of the types above, we simply print the expression
    return {
        "type": "reference",
        "name": full_expression[ast_node.col_offset : ast_node.end_col_offset],
    }


typedoc_types_path = sys.argv[1]

with open(typedoc_types_path, "r") as f:
    typedoc_out = {}
    expressions = json.load(f)

    for expression in expressions:
        try:
            if typedoc_out.get(expression) is None:
                typedoc_out[expression] = parse_expression(
                    ast.parse(expression).body[0].value, expression
                )
        except Exception as e:
            print(f"Invalid expression encountered while parsing: {expression}")
            print(f"Error: {e}")

    with open(f"{os.path.splitext(typedoc_types_path)[0]}-parsed.json", "w") as f:
        f.write(json.dumps(typedoc_out, indent=4))
Repository: apify/crawlee-python
File: src/crawlee/base_storage_client/_models.py (Python)

Code before:
    method: Annotated[HttpMethod | None, Field()] = None


@docs_group('Data structures')
class BatchRequestsOperationResponse(BaseModel):
    """Response to batch request deletion calls."""

    model_config = ConfigDict(populate_by_name=True)

    processed_requests: Annotated[list[ProcessedRequest], Field(alias='processedRequests')]
    unprocessed_requests: Annotated[list[UnprocessedRequest], Field(alias='unprocessedRequests')]


@docs_group('Data structures')
class InternalRequest(BaseModel):
    """Represents an request in queue client."""

    id: str

    unique_key: str

    order_no: Decimal | None = None

    handled_at: datetime | None

    request: Request

    @classmethod
    def from_request(cls, request: Request, id: str, order_no: Decimal | None) -> InternalRequest:
        return cls(
            unique_key=request.unique_key, id=id, handled_at=request.handled_at, order_no=order_no, request=request
        )

    def to_request(self) -> Request:
        return self.request


Review comment (suggestion):
If this could have an alias to `json_`, we wouldn't break compatibility with storage files from older versions, which is not a big deal, but still it would be nice.

Code after:
    url: Annotated[str, BeforeValidator(validate_http_url), Field()]
    method: Annotated[HttpMethod | None, Field()] = None


@docs_group('Data structures')
class BatchRequestsOperationResponse(BaseModel):
    """Response to batch request deletion calls."""

    model_config = ConfigDict(populate_by_name=True)

    processed_requests: Annotated[list[ProcessedRequest], Field(alias='processedRequests')]
    unprocessed_requests: Annotated[list[UnprocessedRequest], Field(alias='unprocessedRequests')]


class InternalRequest(BaseModel):
    """Internal representation of a queue request with additional metadata for ordering and storage."""

    model_config = ConfigDict(populate_by_name=True)

    id: str

    unique_key: str

    order_no: Decimal | None = None
    """Order number for maintaining request sequence in queue.
    Used for restoring correct request order when recovering queue from storage."""

    handled_at: datetime | None

    request: Annotated[
        Request, Field(alias='json_'), BeforeValidator(lambda v: json.loads(v) if isinstance(v, str) else v)
    ]
    """Original Request object. The alias 'json_' is required for backward compatibility with legacy code."""

    @classmethod
    def from_request(cls, request: Request, id: str, order_no: Decimal | None) -> InternalRequest:
        return cls(
            unique_key=request.unique_key, id=id, handled_at=request.handled_at, order_no=order_no, request=request
        )

    def to_request(self) -> Request:
        return self.request
Repository: apify/crawlee-python
File: docs/examples/code/beautifulsoup_crawler.py (Python)

Code before:
import asyncio
from datetime import timedelta

from crawlee._types import BasicCrawlingContext
from crawlee.beautifulsoup_crawler import BeautifulSoupCrawler, BeautifulSoupCrawlingContext


async def main() -> None:
    # Create an instance of the BeautifulSoupCrawler class, a crawler that automatically
    # loads the URLs and parses their HTML using the BeautifulSoup library.
    crawler = BeautifulSoupCrawler(
        # On error, retry each page at most once.
        max_request_retries=1,
        # Increase the timeout for processing each page to 30 seconds.
        request_handler_timeout=timedelta(seconds=30),
        # 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.
    # The handler receives a context parameter, providing various properties and
    # helper methods. Here are a few key ones we use for demonstration:
    # - request: an instance of the Request class containing details such as the URL
    #   being crawled and the HTTP method used.
    # - soup: the BeautifulSoup object containing the parsed HTML of the response.
    @crawler.router.default_handler
    async def request_handler(context: BeautifulSoupCrawlingContext) -> None:
        context.log.info(f'Processing {context.request.url} ...')


Review comment (suggestion):
```suggestion
from crawlee.basic_crawler import BasicCrawlingContext
```

Code after:
import asyncio
from datetime import timedelta

from crawlee.basic_crawler import BasicCrawlingContext
from crawlee.beautifulsoup_crawler import BeautifulSoupCrawler, BeautifulSoupCrawlingContext


async def main() -> None:
    # Create an instance of the BeautifulSoupCrawler class, a crawler that automatically
    # loads the URLs and parses their HTML using the BeautifulSoup library.
    crawler = BeautifulSoupCrawler(
        # On error, retry each page at most once.
        max_request_retries=1,
        # Increase the timeout for processing each page to 30 seconds.
        request_handler_timeout=timedelta(seconds=30),
        # 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.
    # The handler receives a context parameter, providing various properties and
    # helper methods. Here are a few key ones we use for demonstration:
    # - request: an instance of the Request class containing details such as the URL
    #   being crawled and the HTTP method used.
    # - soup: the BeautifulSoup object containing the parsed HTML of the response.
    @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/examples/code/parsel_crawler.py (Python)

Code before:
import asyncio

from crawlee._types import BasicCrawlingContext
from crawlee.parsel_crawler import ParselCrawler, ParselCrawlingContext

# Regex for identifying email addresses on a webpage.
EMAIL_REGEX = r'[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}'


async def main() -> None:
    crawler = ParselCrawler(
        # 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: ParselCrawlingContext) -> None:
        context.log.info(f'Processing {context.request.url} ...')

        # Extract data from the page.
        data = {
            'url': context.request.url,
            'title': context.selector.xpath('//title/text()').get(),
            'email_address_list': context.selector.re(EMAIL_REGEX),
        }

        # Push the extracted data to the default dataset.

Review comment (suggestion):
```suggestion
from crawlee.basic_crawler import BasicCrawlingContext
```

Code after:
import asyncio

from crawlee.basic_crawler import BasicCrawlingContext
from crawlee.parsel_crawler import ParselCrawler, ParselCrawlingContext

# Regex for identifying email addresses on a webpage.
EMAIL_REGEX = r'[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}'


async def main() -> None:
    crawler = ParselCrawler(
        # 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: ParselCrawlingContext) -> None:
        context.log.info(f'Processing {context.request.url} ...')

        # Extract data from the page.
        data = {
            'url': context.request.url,
            'title': context.selector.xpath('//title/text()').get(),
            'email_address_list': context.selector.re(EMAIL_REGEX),
        }

        # Push the extracted data to the default dataset.