From f09a2f0051a7c830277fa60f7a142ebfccb0c205 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Tue, 11 Oct 2022 22:20:56 -0400 Subject: [PATCH] [Fix] only reregister if flag is set (#937) * add test for expected reregister behaviour * add fix * pass passed args into earlier parse * fix test by using args * exit before actual register * use strtobool Co-authored-by: Unconst <32490803+unconst@users.noreply.github.com> --- bittensor/_cli/cli_impl.py | 3 ++- bittensor/_config/__init__.py | 2 +- bittensor/_wallet/__init__.py | 3 ++- tests/integration_tests/test_cli.py | 32 +++++++++++++++++++++++++---- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/bittensor/_cli/cli_impl.py b/bittensor/_cli/cli_impl.py index 0425504486..369da862be 100644 --- a/bittensor/_cli/cli_impl.py +++ b/bittensor/_cli/cli_impl.py @@ -196,7 +196,8 @@ def run_miner ( self ): wallet.coldkeypub # Check registration - self.register() + ## Will exit if --wallet.reregister is False + wallet.reregister() # Run miner. if self.config.model == 'core_server': diff --git a/bittensor/_config/__init__.py b/bittensor/_config/__init__.py index b94e357544..76b4eff293 100644 --- a/bittensor/_config/__init__.py +++ b/bittensor/_config/__init__.py @@ -70,7 +70,7 @@ def __new__( cls, parser: ArgumentParser = None, strict: bool = False, args: Opt # 1.1 Optionally load defaults if the --config is set. try: - config_file_path = str(os.getcwd()) + '/' + vars(parser.parse_known_args()[0])['config'] + config_file_path = str(os.getcwd()) + '/' + vars(parser.parse_known_args(args)[0])['config'] except Exception as e: config_file_path = None diff --git a/bittensor/_wallet/__init__.py b/bittensor/_wallet/__init__.py index 4080ad8cf2..3f83f6b40d 100644 --- a/bittensor/_wallet/__init__.py +++ b/bittensor/_wallet/__init__.py @@ -19,6 +19,7 @@ import argparse import copy +from distutils.util import strtobool import os import bittensor @@ -114,7 +115,7 @@ def add_args(cls, parser: argparse.ArgumentParser, prefix: str = None ): parser.add_argument('--' + prefix_str + 'wallet.hotkeys', '--' + prefix_str + 'wallet.exclude_hotkeys', required=False, action='store', default=bittensor.defaults.wallet.hotkeys, type=str, nargs='*', help='''Specify the hotkeys by name. (e.g. hk1 hk2 hk3)''') parser.add_argument('--' + prefix_str + 'wallet.all_hotkeys', required=False, action='store_true', default=bittensor.defaults.wallet.all_hotkeys, help='''To specify all hotkeys. Specifying hotkeys will exclude them from this all.''') - parser.add_argument('--' + prefix_str + 'wallet.reregister', required=False, action='store', default=bittensor.defaults.wallet.reregister, type=bool, help='''Whether to reregister the wallet if it is not already registered.''') + parser.add_argument('--' + prefix_str + 'wallet.reregister', required=False, default=bittensor.defaults.wallet.reregister, type=lambda x: bool(strtobool(x)), help='''Whether to reregister the wallet if it is not already registered.''') except argparse.ArgumentError as e: pass diff --git a/tests/integration_tests/test_cli.py b/tests/integration_tests/test_cli.py index febbcbf5e7..4b8a7985d4 100644 --- a/tests/integration_tests/test_cli.py +++ b/tests/integration_tests/test_cli.py @@ -1354,8 +1354,32 @@ def test_btcli_help(): assert 'overview' in help_out assert 'run' in help_out +class TestCLIUsingArgs(unittest.TestCase): + """ + Test the CLI by passing args directly to the bittensor.cli factory + """ + def test_run_reregister_false(self): + """ + Verify that the btcli run command does not reregister a not registered wallet + if --wallet.reregister is False + """ + + with patch('bittensor.Wallet.is_registered', MagicMock(return_value=False)) as mock_wallet_is_reg: # Wallet is not registered + with patch('bittensor.Subtensor.register', MagicMock(side_effect=Exception("shouldn't register during test"))): + with pytest.raises(SystemExit): + cli = bittensor.cli(args=[ + 'run', + '--wallet.name', 'mock', + '--wallet.hotkey', 'mock_hotkey', + '--wallet._mock', 'True', + '--subtensor.network', 'mock', + '--subtensor._mock', 'True', + '--no_prompt', + '--wallet.reregister', 'False' # Don't reregister + ]) + cli.run() + + args, kwargs = mock_wallet_is_reg.call_args + # args[0] should be self => the wallet + assert args[0].config.wallet.reregister == False -if __name__ == "__main__": - cli = TestCli() - cli.setUp() - cli.test_stake()