From 9477eecec468c05217e189aa5d46db12dc4ea45a Mon Sep 17 00:00:00 2001 From: "Yuri (solarw) Turchenkov" Date: Thu, 9 Sep 2021 10:01:30 +0300 Subject: [PATCH 1/4] check dependencies present in registry on package push --- aea/cli/registry/push.py | 25 +++++++--- aea/cli/registry/utils.py | 57 ++++++++++++++++------ aea/helpers/http_requests.py | 9 ++++ tests/test_cli/test_registry/test_push.py | 41 ++++++++++++++-- tests/test_cli/test_registry/test_utils.py | 22 ++++++++- 5 files changed, 128 insertions(+), 26 deletions(-) diff --git a/aea/cli/registry/push.py b/aea/cli/registry/push.py index 4133f6126b..17e04490f0 100644 --- a/aea/cli/registry/push.py +++ b/aea/cli/registry/push.py @@ -21,13 +21,14 @@ import os import shutil import tarfile -from typing import cast +from typing import List, Tuple, cast import click from aea.cli.registry.utils import ( check_is_author_logged_in, clean_tarfiles, + list_missing_packages, request_api, ) from aea.cli.utils.context import Context @@ -39,6 +40,7 @@ CONNECTIONS, CONTRACTS, DEFAULT_README_FILE, + ITEM_TYPE_PLURAL_TO_TYPE, PROTOCOLS, SKILLS, ) @@ -143,11 +145,20 @@ def push_item(ctx: Context, item_type: str, item_id: PublicId) -> None: } # dependencies + dependecies: List[Tuple[str, PublicId]] = [] for key in [CONNECTIONS, CONTRACTS, PROTOCOLS, SKILLS]: - deps_list = item_config.get(key) + deps_list = item_config.get(key, []) if deps_list: data.update({key: deps_list}) + for dep in deps_list: + dependecies.append((ITEM_TYPE_PLURAL_TO_TYPE[key], PublicId.from_str(dep))) + missing_dependencies = list_missing_packages(dependecies) + + if missing_dependencies: + for package_type, package_id in missing_dependencies: + click.echo(f"Error: Can not find {package_type} {package_id} in registry!") + raise click.ClickException("Found missed dependencies! Push canceled!") try: files = {"file": open(output_filepath, "rb")} readme_path = os.path.join(item_path, DEFAULT_README_FILE) @@ -159,11 +170,11 @@ def push_item(ctx: Context, item_type: str, item_id: PublicId) -> None: resp = cast( JSONLike, request_api("POST", path, data=data, is_auth=True, files=files) ) + click.echo( + "Successfully pushed {} {} to the Registry. Public ID: {}".format( + item_type, item_id.name, resp["public_id"] + ) + ) finally: for fd in files.values(): fd.close() - click.echo( - "Successfully pushed {} {} to the Registry. Public ID: {}".format( - item_type, item_id.name, resp["public_id"] - ) - ) diff --git a/aea/cli/registry/utils.py b/aea/cli/registry/utils.py index d5b26c5f3c..0ca981f20a 100644 --- a/aea/cli/registry/utils.py +++ b/aea/cli/registry/utils.py @@ -17,11 +17,9 @@ # # ------------------------------------------------------------------------------ """Utils used for operating Registry with CLI.""" - import os import tarfile -from json.decoder import JSONDecodeError -from typing import Any, Callable, Dict, Optional, Tuple, Union, cast +from typing import Any, Callable, Dict, List, Optional, Tuple, Union, cast import click @@ -32,6 +30,7 @@ from aea.cli.utils.package_utils import find_item_locally from aea.common import JSONLike from aea.configurations.base import PublicId +from aea.configurations.constants import ITEM_TYPE_TO_PLURAL from aea.helpers import http_requests as requests @@ -83,21 +82,12 @@ def request_api( 'Please sign in with "aea login" command.' ) headers.update({"Authorization": "Token {}".format(token)}) - - request_kwargs = dict( - method=method, - url="{}{}".format(REGISTRY_API_URL, path), - params=params, - files=files, - data=data, - headers=headers, - ) try: - resp = requests.request(**request_kwargs) + resp = _perform_registry_request(method, path, params, data, files, headers) resp_json = resp.json() except requests.exceptions.ConnectionError: raise click.ClickException("Registry server is not responding.") - except JSONDecodeError: + except requests.JSONDecodeError: resp_json = None if resp.status_code == 200: @@ -128,6 +118,27 @@ def request_api( return resp_json +def _perform_registry_request( + method: str, + path: str, + params: Optional[Dict] = None, + data: Optional[Dict] = None, + files: Optional[Dict] = None, + headers: Optional[Dict] = None, +) -> requests.Response: + """Perform HTTP request and resturn response object.""" + request_kwargs = dict( + method=method, + url="{}{}".format(REGISTRY_API_URL, path), + params=params, + files=files, + data=data, + headers=headers, + ) + resp = requests.request(**request_kwargs) + return resp + + def download_file(url: str, cwd: str, timeout: float = FILE_DOWNLOAD_TIMEOUT) -> str: """ Download file from URL and save it in CWD (current working directory). @@ -311,3 +322,21 @@ def get_latest_version_available_in_registry( ) return latest_item_public_id + + +def list_missing_packages( + packages: List[Tuple[str, PublicId]] +) -> List[Tuple[str, PublicId]]: + """Get list of packages do not present in registry.""" + result: List[Tuple[str, PublicId]] = [] + + for package_type, package_id in packages: + api_path = f"/{ITEM_TYPE_TO_PLURAL[package_type]}/{package_id.author}/{package_id.name}/{package_id.version}" + resp = _perform_registry_request("GET", api_path) + if resp.status_code == 404: + result.append((package_type, package_id)) + elif resp.status_code == 200: + pass + else: # pragma: nocover + raise ValueError("Error on registry request") + return result diff --git a/aea/helpers/http_requests.py b/aea/helpers/http_requests.py index 5d75e2377f..21d90ead0a 100644 --- a/aea/helpers/http_requests.py +++ b/aea/helpers/http_requests.py @@ -28,6 +28,15 @@ DEFAULT_TIMEOUT = NETWORK_REQUEST_DEFAULT_TIMEOUT +# requests can use one of these +try: + from simplejson.errors import ( # type: ignore # pylint: disable=unused-import + JSONDecodeError, + ) +except ModuleNotFoundError: # pragma: nocover + from json.decoder import JSONDecodeError # noqa # pylint: disable=unused-import + + def add_default_timeout(fn: Callable, timeout: float) -> Callable: """Add default timeout for requests methods.""" diff --git a/tests/test_cli/test_registry/test_push.py b/tests/test_cli/test_registry/test_push.py index bc6b58ce61..42b716a4ee 100644 --- a/tests/test_cli/test_registry/test_push.py +++ b/tests/test_cli/test_registry/test_push.py @@ -19,7 +19,7 @@ """Test module for Registry push methods.""" import os from unittest import TestCase, mock -from unittest.mock import mock_open +from unittest.mock import mock_open, patch import pytest from click import ClickException @@ -37,6 +37,7 @@ @mock.patch("builtins.open", mock_open(read_data="opened_file")) @mock.patch("aea.cli.registry.push.check_is_author_logged_in") +@mock.patch("aea.cli.registry.push.list_missing_packages", return_value=[]) @mock.patch("aea.cli.registry.utils._rm_tarfiles") @mock.patch("aea.cli.registry.push.os.getcwd", return_value="cwd") @mock.patch("aea.cli.registry.push._compress_dir") @@ -47,7 +48,7 @@ "version": PublicIdMock.DEFAULT_VERSION, "author": "some_author", "name": "some_name", - "protocols": ["protocol_id"], + "protocols": ["some/protocol:0.1.2"], }, ) @mock.patch( @@ -68,6 +69,7 @@ def test_push_item_positive( getcwd_mock, rm_tarfiles_mock, check_is_author_logged_in_mock, + *_ ): """Test for push_item positive result.""" public_id = PublicIdMock( @@ -83,12 +85,42 @@ def test_push_item_positive( "name": "some_name", "description": "some-description", "version": PublicIdMock.DEFAULT_VERSION, - "protocols": ["protocol_id"], + "protocols": ["some/protocol:0.1.2"], }, is_auth=True, files={"file": open("file.1"), "readme": open("file.2")}, ) + @mock.patch("aea.cli.registry.push.os.path.exists", return_value=True) + @mock.patch("aea.cli.registry.push.is_readme_present", return_value=True) + def test_push_dependency_fail( + self, + is_readme_present_mock, + path_exists_mock, + request_api_mock, + load_yaml_mock, + compress_mock, + getcwd_mock, + rm_tarfiles_mock, + check_is_author_logged_in_mock, + *_ + ): + """Test for push_item fails cause dependencies check.""" + public_id = PublicIdMock( + name="some_name", + author="some_author", + version="{}".format(PublicIdMock.DEFAULT_VERSION), + ) + + with patch( + "aea.cli.registry.push.list_missing_packages", + return_value=[("some", PublicId.from_str("some/pack:0.1.0"))], + ): + with pytest.raises( + ClickException, match="Found missed dependencies! Push canceled!" + ): + push_item(ContextMock(), "some-type", public_id) + @mock.patch("aea.cli.registry.push.os.path.exists", return_value=True) @mock.patch("aea.cli.registry.push.is_readme_present", return_value=False) def test_push_item_positive_without_readme( @@ -108,7 +140,7 @@ def test_push_item_positive_without_readme( "name": "some_name", "description": "some-description", "version": PublicIdMock.DEFAULT_VERSION, - "protocols": ["protocol_id"], + "protocols": ["some/protocol:0.1.2"], }, is_auth=True, files={"file": open("opened_file", "r")}, @@ -124,6 +156,7 @@ def test_push_item_item_not_found( getcwd_mock, rm_tarfiles_mock, check_is_author_logged_in_mock, + *_ ): """Test for push_item - item not found.""" with self.assertRaises(ClickException): diff --git a/tests/test_cli/test_registry/test_utils.py b/tests/test_cli/test_registry/test_utils.py index a8eec1a7cc..158dc39cc4 100644 --- a/tests/test_cli/test_registry/test_utils.py +++ b/tests/test_cli/test_registry/test_utils.py @@ -23,7 +23,7 @@ from json.decoder import JSONDecodeError from pathlib import Path from unittest import TestCase, mock -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest from click import ClickException @@ -41,6 +41,7 @@ get_latest_version_available_in_registry, get_package_meta, is_auth_token_present, + list_missing_packages, request_api, ) from aea.cli.utils.exceptions import AEAConfigException @@ -394,3 +395,22 @@ def test_get_latest_version_available_in_registry_remote_mode(*_mocks): context_mock, "protocol", DefaultMessage.protocol_id ) assert result == DefaultMessage.protocol_id + + +def test_list_missing_packages(): + """Test 'list_missing_packages'.""" + packages = [("connection", PublicId.from_str("test/test:0.1.2"))] + + resp_ok = MagicMock() + resp_ok.status_code = 200 + with patch( + "aea.cli.registry.utils._perform_registry_request", return_value=resp_ok + ): + assert list_missing_packages(packages) == [] + + resp_404 = MagicMock() + resp_404.status_code = 404 + with patch( + "aea.cli.registry.utils._perform_registry_request", return_value=resp_404 + ): + assert list_missing_packages(packages) == packages From 2ffe0029aabb622b6afb767e77bf364663fabf3e Mon Sep 17 00:00:00 2001 From: David Minarsch Date: Sat, 18 Sep 2021 11:16:16 +0100 Subject: [PATCH 2/4] Apply suggestions from code review --- aea/cli/registry/push.py | 6 +++--- aea/cli/registry/utils.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/aea/cli/registry/push.py b/aea/cli/registry/push.py index 17e04490f0..4d55920421 100644 --- a/aea/cli/registry/push.py +++ b/aea/cli/registry/push.py @@ -145,20 +145,20 @@ def push_item(ctx: Context, item_type: str, item_id: PublicId) -> None: } # dependencies - dependecies: List[Tuple[str, PublicId]] = [] + dependencies: List[Tuple[str, PublicId]] = [] for key in [CONNECTIONS, CONTRACTS, PROTOCOLS, SKILLS]: deps_list = item_config.get(key, []) if deps_list: data.update({key: deps_list}) for dep in deps_list: - dependecies.append((ITEM_TYPE_PLURAL_TO_TYPE[key], PublicId.from_str(dep))) + dependencies.append((ITEM_TYPE_PLURAL_TO_TYPE[key], PublicId.from_str(dep))) missing_dependencies = list_missing_packages(dependecies) if missing_dependencies: for package_type, package_id in missing_dependencies: click.echo(f"Error: Can not find {package_type} {package_id} in registry!") - raise click.ClickException("Found missed dependencies! Push canceled!") + raise click.ClickException("Found missing dependencies! Push canceled!") try: files = {"file": open(output_filepath, "rb")} readme_path = os.path.join(item_path, DEFAULT_README_FILE) diff --git a/aea/cli/registry/utils.py b/aea/cli/registry/utils.py index 0ca981f20a..f134a9940b 100644 --- a/aea/cli/registry/utils.py +++ b/aea/cli/registry/utils.py @@ -327,7 +327,7 @@ def get_latest_version_available_in_registry( def list_missing_packages( packages: List[Tuple[str, PublicId]] ) -> List[Tuple[str, PublicId]]: - """Get list of packages do not present in registry.""" + """Get list of packages not currently present in registry.""" result: List[Tuple[str, PublicId]] = [] for package_type, package_id in packages: From 8bff8f39611d8b5afa9dd960e058725b843f43eb Mon Sep 17 00:00:00 2001 From: David Minarsch Date: Sat, 18 Sep 2021 16:52:08 +0100 Subject: [PATCH 3/4] Update aea/cli/registry/push.py Co-authored-by: Marco Favorito --- aea/cli/registry/push.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aea/cli/registry/push.py b/aea/cli/registry/push.py index 4d55920421..05e5767ea1 100644 --- a/aea/cli/registry/push.py +++ b/aea/cli/registry/push.py @@ -157,7 +157,7 @@ def push_item(ctx: Context, item_type: str, item_id: PublicId) -> None: if missing_dependencies: for package_type, package_id in missing_dependencies: - click.echo(f"Error: Can not find {package_type} {package_id} in registry!") + click.echo(f"Error: Cannot find {package_type} {package_id} in registry!") raise click.ClickException("Found missing dependencies! Push canceled!") try: files = {"file": open(output_filepath, "rb")} From 154b8b037a086840d9e2d7cca29eb80cfd9b68e9 Mon Sep 17 00:00:00 2001 From: David Minarsch Date: Sun, 19 Sep 2021 20:34:02 +0000 Subject: [PATCH 4/4] fix: spelling --- aea/cli/registry/push.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aea/cli/registry/push.py b/aea/cli/registry/push.py index 05e5767ea1..7bedd5325d 100644 --- a/aea/cli/registry/push.py +++ b/aea/cli/registry/push.py @@ -153,7 +153,7 @@ def push_item(ctx: Context, item_type: str, item_id: PublicId) -> None: for dep in deps_list: dependencies.append((ITEM_TYPE_PLURAL_TO_TYPE[key], PublicId.from_str(dep))) - missing_dependencies = list_missing_packages(dependecies) + missing_dependencies = list_missing_packages(dependencies) if missing_dependencies: for package_type, package_id in missing_dependencies: