Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix misleading message on generate-key #681

Merged
merged 5 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions aea/cli/generate_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import click

from aea.cli.add_key import _add_private_key
from aea.cli.utils.click_utils import password_option
from aea.cli.utils.click_utils import LedgerChoice, password_option
from aea.cli.utils.decorators import _check_aea_project
from aea.configurations.constants import (
ADDRESS,
Expand All @@ -41,7 +41,7 @@
@click.argument(
"type_",
metavar="TYPE",
type=click.Choice([*list(crypto_registry.supported_ids), "all"]),
type=LedgerChoice(),
required=True,
)
@click.argument(
Expand Down
30 changes: 29 additions & 1 deletion aea/cli/utils/click_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#
# ------------------------------------------------------------------------------
"""Module with click utils of the aea cli."""

import os
from collections import OrderedDict
from contextlib import contextmanager
Expand Down Expand Up @@ -46,6 +45,7 @@
SKILL,
)
from aea.configurations.data_types import Dependency, PackageType, PublicId
from aea.crypto.registries import crypto_registry
from aea.helpers.io import open_file
from aea.package_manager.base import PACKAGE_SOURCE_RE

Expand Down Expand Up @@ -259,6 +259,34 @@ def convert(self, value: str, param: Any, ctx: click.Context) -> Dependency:
raise click.ClickException(str(e)) from e


class LedgerChoice(click.ParamType):
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice utility!

"""Ledger choice flag."""

ALL = "all"

def get_metavar(self, param: click.Parameter) -> str:
"""Get metavar name."""
return "LEDGER_ID"

def convert(
self,
value: str,
param: Optional[click.Parameter] = None,
ctx: Optional[click.Context] = None,
) -> str:
"""Convert to ledger id."""
if len(crypto_registry.supported_ids) == 0:
raise click.ClickException("No ledger installation found")

if value not in crypto_registry.supported_ids and value != self.ALL:
raise click.ClickException(
f"Invalid identifier provided `{value}`; "
f"Available ledger identifiers {crypto_registry.supported_ids}"
)
Comment on lines +278 to +285
Copy link
Author

Choose a reason for hiding this comment

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

Better error messages


return value


def registry_flag(
mark_default: bool = True,
default_registry: Optional[str] = None,
Expand Down
23 changes: 21 additions & 2 deletions tests/test_cli/test_generate_key.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# -*- coding: utf-8 -*-
# ------------------------------------------------------------------------------
#
# Copyright 2022 Valory AG
# Copyright 2022-2023 Valory AG
# Copyright 2018-2021 Fetch.AI Limited
#
# Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -38,7 +38,7 @@
from aea.cli import cli
from aea.configurations.constants import MULTIKEY_FILENAME
from aea.crypto.base import Crypto as BaseCrypto
from aea.crypto.registries import make_crypto
from aea.crypto.registries import crypto_registry, make_crypto
from aea.helpers.io import open_file
from aea.helpers.sym_link import cd
from aea.test_tools.test_cases import AEATestCaseEmpty
Expand Down Expand Up @@ -105,6 +105,25 @@ def test_all(self):
Path(FETCHAI_PRIVATE_KEY_FILE).unlink()
Path(ETHEREUM_PRIVATE_KEY_FILE).unlink()

def test_invalid_ledger_id(self):
"""Test that the fetch private key is created correctly."""
args = [*CLI_LOG_OPTION, "generate-key", "ledger"]
result = self.runner.invoke(cli, args)
assert result.exit_code == 1
assert "Invalid identifier provided `ledger`" in result.stdout

def test_no_ledger_installation_found(self):
"""Test that the fetch private key is created correctly."""
args = [*CLI_LOG_OPTION, "generate-key", "ledger"]
specs = crypto_registry.specs.copy()
crypto_registry.specs = {}
try:
result = self.runner.invoke(cli, args)
assert result.exit_code == 1
assert "No ledger installation found" in result.stdout
finally:
crypto_registry.specs = specs

@classmethod
def teardown_class(cls):
"""Tear the test down."""
Expand Down
Loading