ronantakizawa/codereview-bench huggingface dataset:
Repository: jina-ai/jina
File: jina/hubble/helper.py (Python)

Code before:
"""Module for helper functions for Hub API."""

import hashlib
import io
import json
import os
import shelve
import subprocess
import sys
import tarfile
import urllib
import warnings
import zipfile
from contextlib import nullcontext
from functools import lru_cache, wraps
from pathlib import Path
from typing import Dict, Optional, Tuple
from urllib.parse import urljoin, urlparse

from jina import __resources_path__
from jina.enums import BetterEnum
from jina.helper import get_request_header as _get_request_header_main
from jina.importer import ImportExtensions
from jina.logging.predefined import default_logger
from .requirements import (
    get_env_variables,
    check_env_variable,
    parse_requirement,
    expand_env_variables
)


@lru_cache()
def _get_hub_root() -> Path:
    hub_root = Path(os.environ.get('JINA_HUB_ROOT', Path.home().joinpath('.jina')))

    if not hub_root.exists():
        hub_root.mkdir(parents=True, exist_ok=True)

    return hub_root


@lru_cache()
def _get_hub_config() -> Optional[Dict]:
    hub_root = _get_hub_root()

    config_file = hub_root.joinpath('config.json')
    if config_file.exists():
        with open(config_file) as f:
            return json.load(f)

Review comment (suggestion):
```suggestion
from jina.hubble.requirements import (
```

Code after:
"""Module for helper functions for Hub API."""

import hashlib
import io
import json
import os
import shelve
import subprocess
import sys
import tarfile
import urllib
import warnings
import zipfile
from contextlib import nullcontext
from functools import lru_cache, wraps
from pathlib import Path
from typing import Dict, Optional, Tuple
from urllib.parse import urljoin, urlparse

from jina import __resources_path__
from jina.enums import BetterEnum
from jina.helper import get_request_header as _get_request_header_main
from jina.importer import ImportExtensions
from jina.logging.predefined import default_logger
from jina.hubble.requirements import (
    get_env_variables,
    check_env_variable,
    parse_requirement,
    expand_env_variables
)


@lru_cache()
def _get_hub_root() -> Path:
    hub_root = Path(os.environ.get('JINA_HUB_ROOT', Path.home().joinpath('.jina')))

    if not hub_root.exists():
        hub_root.mkdir(parents=True, exist_ok=True)

    return hub_root


@lru_cache()
def _get_hub_config() -> Optional[Dict]:
    hub_root = _get_hub_root()

    config_file = hub_root.joinpath('config.json')
    if config_file.exists():
        with open(config_file) as f:
            return json.load(f)
Repository: jina-ai/jina
File: jina/hubble/helper.py (Python)

Code before:
import zipfile
from contextlib import nullcontext
from functools import lru_cache, wraps
from pathlib import Path
from typing import Dict, Optional, Tuple
from urllib.parse import urljoin, urlparse

from jina import __cache_path__, __resources_path__
from jina.enums import BetterEnum
from jina.helper import get_request_header as _get_request_header_main
from jina.hubble.requirements import (
    check_env_variable,
    expand_env_variables,
    get_env_variables,
    parse_requirement,
)
from jina.importer import ImportExtensions
from jina.logging.predefined import default_logger


@lru_cache()
def _get_hub_root() -> Path:
    hub_root = Path(os.environ.get('JINA_HUB_ROOT', __cache_path__))

    if not hub_root.exists():
        hub_root.mkdir(parents=True, exist_ok=True)

    return hub_root


@lru_cache()
def _get_hub_config() -> Optional[Dict]:
    hub_root = _get_hub_root()

    config_file = hub_root.joinpath('config.json')
    if config_file.exists():
        with open(config_file) as f:
            return json.load(f)


@lru_cache()
def get_hub_packages_dir() -> Path:
    """Get the path of folder where the hub packages are stored

    :return: the path of folder where the hub packages are stored
    """
    root = _get_hub_root()
    hub_packages = root.joinpath('hub-package')

    if not hub_packages.exists():
        hub_packages.mkdir(parents=True, exist_ok=True)

Review comment (performance):
I guess this will deprecate `JINA_HUB_CACHE_DIR` and `JINA_HUB_ROOT`. If we really don't want them anymore, then let's remove them from the code base, docs and tests

Code after:
from contextlib import nullcontext
from functools import lru_cache, wraps
from pathlib import Path
from typing import Dict, Optional, Tuple
from urllib.parse import urlparse

from jina import __cache_path__, __resources_path__
from jina.enums import BetterEnum
from jina.helper import get_request_header as _get_request_header_main
from jina.hubble.requirements import (
    check_env_variable,
    expand_env_variables,
    get_env_variables,
    parse_requirement,
)
from jina.importer import ImportExtensions
from jina.logging.predefined import default_logger


@lru_cache()
def get_hub_packages_dir() -> Path:
    """Get the path of folder where the hub packages are stored

    :return: the path of folder where the hub packages are stored
    """
    root = Path(__cache_path__)
    hub_packages = root.joinpath('hub-package')

    if not hub_packages.exists():
        hub_packages.mkdir(parents=True, exist_ok=True)

    return hub_packages


@lru_cache()
def get_cache_db() -> Path:
    """Get the path of cache db of hub Executors

    :return: the path of cache db of hub Executors
    """
    root = Path(__cache_path__)
    cache_db = root.joinpath('disk_cache.db')

    return cache_db


@lru_cache()
def get_download_cache_dir() -> Path:
    """Get the path of cache folder where the downloading cache is stored

    :return: the path of cache folder where the downloading cache is stored
Repository: jina-ai/jina
File: jina/parsers/ping.py (Python)

Code before:
    parser.add_argument(
        'host',
        type=str,
        help='The host address with port of a target Executor, Gateway or a Flow, e.g. 0.0.0.0:8000. For Flow or Gateway, host can also indicate the protocol, grpc will be used if non provided, e.g http://0.0.0.0:8000',
    )

    parser.add_argument(
        '--timeout',
        type=int,
        default=3000,
        help='''
Timeout in millisecond of one check
-1 for waiting forever
''',
    )
    parser.add_argument(
        '--attempts',
        type=int,
        default=1,
        help='The number of readiness checks to perform',
    )
    parser.add_argument(
        '--min-successful-attempts',
        type=int,
        default=1,
        help='The minimum number of successful readiness checks, before it exits successfully with exit(0)',
    )
    return parser


Review comment (suggestion):
```suggestion
        help='The minimum number of successful readiness checks, before exiting successfully with exit(0)',
```

Code after:
    parser.add_argument(
        'host',
        type=str,
        help='The host address with port of a target Executor, Gateway or a Flow, e.g. 0.0.0.0:8000. For Flow or Gateway, host can also indicate the protocol, grpc will be used if not provided, e.g http://0.0.0.0:8000',
    )

    parser.add_argument(
        '--timeout',
        type=int,
        default=3000,
        help='''
Timeout in millisecond of one check
-1 for waiting forever
''',
    )
    parser.add_argument(
        '--attempts',
        type=int,
        default=1,
        help='The number of readiness checks to perform',
    )
    parser.add_argument(
        '--min-successful-attempts',
        type=int,
        default=1,
        help='The minimum number of successful readiness checks, before exiting successfully with exit(0)',
    )
    return parser
Repository: jina-ai/jina
File: jina/jaml/parsers/__init__.py (Python)

Code before:
import warnings
from typing import List, Optional, Type

from jina.excepts import BadYAMLVersion
from jina.jaml import JAMLCompatible
from jina.jaml.parsers.base import VersionedYAMLParser
from jina.serve.gateway import BaseGateway


def _get_all_parser(cls: Type['JAMLCompatible']):
    """Get all parsers and legacy parser of a class

    :param cls: target class
    :return: a tuple of two elements; first is a list of all parsers, second is the legacy parser for default fallback
    """
    from jina.orchestrate.flow.base import Flow
    from jina.serve.executors import BaseExecutor

    if issubclass(cls, Flow):
        return _get_flow_parser()
    elif issubclass(cls, BaseExecutor):
        return _get_exec_parser()
    elif issubclass(cls, BaseGateway):
        return _get_gateway_parser()


def _get_flow_parser():
    from jina.jaml.parsers.flow.v1 import V1Parser

    return [V1Parser], V1Parser


def _get_exec_parser():
    from jina.jaml.parsers.executor.legacy import ExecutorLegacyParser

    return [ExecutorLegacyParser], ExecutorLegacyParser


def _get_gateway_parser():
    from jina.jaml.parsers.gateway.legacy import GatewayLegacyParser

    return [GatewayLegacyParser], GatewayLegacyParser


def get_parser(
    cls: Type['JAMLCompatible'], version: Optional[str]
) -> 'VersionedYAMLParser':
    """



Review comment (suggestion):
else, raise error.
Ideally, each JAML compatible class should select its parser rather than having to add code to select it (but not so important for now)

Code after:
import warnings
from typing import List, Optional, Type

from jina.excepts import BadYAMLVersion
from jina.jaml import JAMLCompatible
from jina.jaml.parsers.base import VersionedYAMLParser
from jina.serve.gateway import BaseGateway


def _get_all_parser(cls: Type['JAMLCompatible']):
    """Get all parsers and legacy parser of a class

    :param cls: target class
    :return: a tuple of two elements; first is a list of all parsers, second is the legacy parser for default fallback
    """
    from jina.orchestrate.flow.base import Flow
    from jina.serve.executors import BaseExecutor

    if issubclass(cls, Flow):
        return _get_flow_parser()
    elif issubclass(cls, BaseExecutor):
        return _get_exec_parser()
    elif issubclass(cls, BaseGateway):
        return _get_gateway_parser()
    else:
        raise NotImplementedError(f'No parser exists for cls {cls.__name__}')


def _get_flow_parser():
    from jina.jaml.parsers.flow.v1 import V1Parser

    return [V1Parser], V1Parser


def _get_exec_parser():
    from jina.jaml.parsers.executor.legacy import ExecutorLegacyParser

    return [ExecutorLegacyParser], ExecutorLegacyParser


def _get_gateway_parser():
    from jina.jaml.parsers.gateway.legacy import GatewayLegacyParser

    return [GatewayLegacyParser], GatewayLegacyParser


def get_parser(
    cls: Type['JAMLCompatible'], version: Optional[str]
) -> 'VersionedYAMLParser':
    """
Repository: jina-ai/jina
File: tests/unit/serve/instrumentation/test_gateway_metric_labels.py (Python)

Code before:

    f = (
        Flow(
            tracing=False,
            metrics=True,
            metrics_exporter_host='localhost',
            metrics_exporter_port=4317,
            port=12345,
        )
        .add(name='name1')
        .add(name='name2', needs=['gateway'])
        .add(name='name3', needs=['name1', 'name2'], disable_reduce=True)
    )
    with f:
        f.post('/')
        collect_metrics()
        metrics = read_metrics()

        gateway_metrics = metrics['gateway/rep-0'][0]['resource_metrics'][0][
            'scope_metrics'
        ][0]['metrics']
        gateway_metric_data_point = {
            i['name']: i['data']['data_points'] for i in gateway_metrics
        }

        assert {'name1', 'name2', 'name3'} == {
            i['attributes']['deployment']
            for i in gateway_metric_data_point['jina_received_response_bytes']
        }
        assert {'name1', 'name2', 'name3'} == {
            i['attributes']['deployment']
            for i in gateway_metric_data_point['jina_sent_request_bytes']
        }
        assert {'name1', 'name2', 'name3'} == {
            i['attributes']['deployment']
            for i in gateway_metric_data_point['jina_sending_request_seconds']
        }


Review comment (suggestion):
can u do these assertions outside the context manager? I believe, if there is an error and ctxt manager is left, it may not be called

Code after:

    f = (
        Flow(
            tracing=False,
            metrics=True,
            metrics_exporter_host='localhost',
            metrics_exporter_port=4317,
            port=12345,
        )
        .add(name='name1')
        .add(name='name2', needs=['gateway'])
        .add(name='name3', needs=['name1', 'name2'], disable_reduce=True)
    )
    with f:
        f.post('/')
        collect_metrics()
        metrics = read_metrics()

        gateway_metrics = metrics['gateway/rep-0'][0]['resource_metrics'][0][
            'scope_metrics'
        ][0]['metrics']
        gateway_metric_data_point = {
            i['name']: i['data']['data_points'] for i in gateway_metrics
        }

    assert {'name1', 'name2', 'name3'} == {
        i['attributes']['deployment']
        for i in gateway_metric_data_point['jina_received_response_bytes']
    }
    assert {'name1', 'name2', 'name3'} == {
        i['attributes']['deployment']
        for i in gateway_metric_data_point['jina_sent_request_bytes']
    }
    assert {'name1', 'name2', 'name3'} == {
        i['attributes']['deployment']
        for i in gateway_metric_data_point['jina_sending_request_seconds']
    }
Repository: jina-ai/jina
File: jina/parsers/orchestrate/runtimes/worker.py (Python)

Code before:
    gp.add_argument(
        '--output-array-type',
        type=str,
        default=None,
        help='''
The type of array `tensor` and `embedding` will be serialized to.

Supports the same types as `docarray.to_protobuf(.., ndarray_type=...)`, which can be found 
`here <https://docarray.jina.ai/fundamentals/document/serialization/#from-to-protobuf>`.
Defaults to retaining whatever type is returned by the Executor.
''',
    )

    gp.add_argument(
        '--exit-on-exceptions',
        type=str,
        default=[],
        nargs='*',
        help='List of exceptions that will cause the Executor to shut down.',
    )

    gp.add_argument(
        '--disable-reduce',
        action='store_true',
        default=False,
        help='Disable the built-in reduce mechanism, set this if the reduction is to be handled by the Executor, if the Executor is expected to handle `docs_matrix`',
    )

    gp.add_argument(
        '--docs-by-executor',
        action='store_true',
        default=False,
        help='If enabled, in combination with `--disable-reduce`,the `docs_matrix` received by Executor will be a dictionary with previous Executor name in the key and docs as values',
    )

    mixin_base_runtime_parser(gp)


Review comment (suggestion):
```suggestion
        help='Disable the built-in reduction mechanism. Set this if the reduction is to be handled by the Executor itself by operating on a `docs_matrix`',
```

Code after:
    gp.add_argument(
        '--output-array-type',
        type=str,
        default=None,
        help='''
The type of array `tensor` and `embedding` will be serialized to.

Supports the same types as `docarray.to_protobuf(.., ndarray_type=...)`, which can be found 
`here <https://docarray.jina.ai/fundamentals/document/serialization/#from-to-protobuf>`.
Defaults to retaining whatever type is returned by the Executor.
''',
    )

    gp.add_argument(
        '--exit-on-exceptions',
        type=str,
        default=[],
        nargs='*',
        help='List of exceptions that will cause the Executor to shut down.',
    )

    gp.add_argument(
        '--no-reduce',
        '--disable-reduce',
        action='store_true',
        default=False,
        help='Disable the built-in reduction mechanism. Set this if the reduction is to be handled by the Executor itself by operating on a `docs_matrix` or `docs_map`',
    )

    mixin_base_runtime_parser(gp)
Repository: jina-ai/jina
File: jina/types/request/data.py (Python)

Code before:
        return Response(request=self.proto)

    @property
    def status(self):
        """
        Returns the status from the header field

        :return: the status object of this request
        """
        return self.proto_wo_data.header.status

    @property
    def last_executor(self):
        """
        Returns the name of the last Executor that has processed this Request

        :return: the name of the last Executor that processed this Request
        """
        if len(self.proto_wo_data.routes) > 0:
            return self.proto_wo_data.routes[-1].executor

    def add_executor(self, executor_name: str):
        """
        Returns the name of the last Executor that has processed this Request

        :param executor_name: name of the Executor processing the Request
        """
        route_proto = jina_pb2.RouteProto()
        route_proto.executor = executor_name
        self.proto_wo_data.routes.append(route_proto)

    @property
    def routes(self):
        """
        Returns the routes from the request

        :return: the routes object of this request
        """
        return self.proto_wo_data.routes

    @property
    def request_id(self):
        """
        Returns the request_id from the header field

        :return: the request_id object of this request
        """
        return self.proto.header.request_id

    @classmethod
    def from_proto(cls, request: 'jina_pb2.DataRequestProto'):

Review comment (suggestion):
```suggestion
        :param executor_name: name of the Executor processing the Request to be added to the routes
```

Code after:
        return Response(request=self.proto)

    @property
    def status(self):
        """
        Returns the status from the header field

        :return: the status object of this request
        """
        return self.proto_wo_data.header.status

    @property
    def last_executor(self):
        """
        Returns the name of the last Executor that has processed this Request

        :return: the name of the last Executor that processed this Request
        """
        if len(self.proto_wo_data.routes) > 0:
            return self.proto_wo_data.routes[-1].executor

    def add_executor(self, executor_name: str):
        """
        Adds Executor the the request routes

        :param executor_name: name of the Executor processing the Request to be added to the routes
        """
        route_proto = jina_pb2.RouteProto()
        route_proto.executor = executor_name
        print(f' type {type(self.proto_wo_data.routes)}')
        self.proto_wo_data.routes.append(route_proto)

    @property
    def routes(self):
        """
        Returns the routes from the request

        :return: the routes object of this request
        """
        return self.proto_wo_data.routes

    @property
    def request_id(self):
        """
        Returns the request_id from the header field

        :return: the request_id object of this request
        """
        return self.proto.header.request_id

    @classmethod
Repository: jina-ai/jina
File: jina/types/request/data.py (Python)

Code before:
        :return: the response of this request (self) as an instance of Response
        """
        return Response(request=self.proto)

    @property
    def status(self):
        """
        Returns the status from the header field

        :return: the status object of this request
        """
        return self.proto_wo_data.header.status

    @property
    def last_executor(self):
        """
        Returns the name of the last Executor that has processed this Request

        :return: the name of the last Executor that processed this Request
        """
        if len(self.proto_wo_data.routes) > 0:
            return self.proto_wo_data.routes[-1].executor

    def add_executor(self, executor_name: str):
        """
        Returns the name of the last Executor that has processed this Request

        :param executor_name: name of the Executor processing the Request to be added to the routes
        """
        route_proto = jina_pb2.RouteProto()
        route_proto.executor = executor_name
        self.proto_wo_data.routes.append(route_proto)

    @property
    def routes(self):
        """
        Returns the routes from the request

        :return: the routes object of this request
        """
        return self.proto_wo_data.routes

    @property
    def request_id(self):
        """
        Returns the request_id from the header field

        :return: the request_id object of this request
        """
        return self.proto.header.request_id


Review comment (suggestion):
```suggestion
        Adds Executor the the request routes
```

Code after:
        :return: the response of this request (self) as an instance of Response
        """
        return Response(request=self.proto)

    @property
    def status(self):
        """
        Returns the status from the header field

        :return: the status object of this request
        """
        return self.proto_wo_data.header.status

    @property
    def last_executor(self):
        """
        Returns the name of the last Executor that has processed this Request

        :return: the name of the last Executor that processed this Request
        """
        if len(self.proto_wo_data.routes) > 0:
            return self.proto_wo_data.routes[-1].executor

    def add_executor(self, executor_name: str):
        """
        Adds Executor the the request routes

        :param executor_name: name of the Executor processing the Request to be added to the routes
        """
        route_proto = jina_pb2.RouteProto()
        route_proto.executor = executor_name
        print(f' type {type(self.proto_wo_data.routes)}')
        self.proto_wo_data.routes.append(route_proto)

    @property
    def routes(self):
        """
        Returns the routes from the request

        :return: the routes object of this request
        """
        return self.proto_wo_data.routes

    @property
    def request_id(self):
        """
        Returns the request_id from the header field

        :return: the request_id object of this request
        """
        return self.proto.header.request_id
Repository: jina-ai/jina
File: jina/parsers/orchestrate/runtimes/worker.py (Python)

Code before:
    gp.add_argument(
        '--output-array-type',
        type=str,
        default=None,
        help='''
The type of array `tensor` and `embedding` will be serialized to.

Supports the same types as `docarray.to_protobuf(.., ndarray_type=...)`, which can be found 
`here <https://docarray.jina.ai/fundamentals/document/serialization/#from-to-protobuf>`.
Defaults to retaining whatever type is returned by the Executor.
''',
    )

    gp.add_argument(
        '--exit-on-exceptions',
        type=str,
        default=[],
        nargs='*',
        help='List of exceptions that will cause the Executor to shut down.',
    )

    gp.add_argument(
        '--disable-reduce',
        action='store_true',
        default=False,
        help='Disable the built-in reduction mechanism. Set this if the reduction is to be handled by the Executor itself by operating on a `docs_matrix`',
    )

    mixin_base_runtime_parser(gp)


Review comment (suggestion):
```suggestion
        help='Disable the built-in reduction mechanism. Set this if the reduction is to be handled by the Executor itself by operating on a `docs_matrix` or `docs_map`',
```

Code after:
    gp.add_argument(
        '--output-array-type',
        type=str,
        default=None,
        help='''
The type of array `tensor` and `embedding` will be serialized to.

Supports the same types as `docarray.to_protobuf(.., ndarray_type=...)`, which can be found 
`here <https://docarray.jina.ai/fundamentals/document/serialization/#from-to-protobuf>`.
Defaults to retaining whatever type is returned by the Executor.
''',
    )

    gp.add_argument(
        '--exit-on-exceptions',
        type=str,
        default=[],
        nargs='*',
        help='List of exceptions that will cause the Executor to shut down.',
    )

    gp.add_argument(
        '--no-reduce',
        '--disable-reduce',
        action='store_true',
        default=False,
        help='Disable the built-in reduction mechanism. Set this if the reduction is to be handled by the Executor itself by operating on a `docs_matrix` or `docs_map`',
    )

    mixin_base_runtime_parser(gp)