Skip to content

Commit

Permalink
improve cmd deprecation warning by support nested replaced_by_cmd
Browse files Browse the repository at this point in the history
  • Loading branch information
@mjurbanski-reef
mjurbanski-reef committed Apr 25, 2024
1 parent 6146f70 commit 589d49a
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 38 deletions.
36 changes: 21 additions & 15 deletions b2/_internal/console_tool.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1121,12 +1121,12 @@ def __str__(self):

class CmdReplacedByMixin:
deprecated = True
replaced_by_cmd: type[Command]
replaced_by_cmd: type[Command] | tuple[type[Command], ...]

def run(self, args):
self._print_stderr(
f'WARNING: {self.__class__.name_and_alias()[0]} command is deprecated. '
f'Use {self.replaced_by_cmd.name_and_alias()[0]} instead.'
f'WARNING: `{self.__class__.name_and_alias()[0]}` command is deprecated. '
f'Use `{self.get_replaced_command_name()}` instead.'
)
return super().run(args)

Expand All @@ -1135,9 +1135,15 @@ def _get_description(cls, **kwargs):
return (
f'{super()._get_description(**kwargs)}\n\n'
f'.. warning::\n'
f' This command is deprecated. Use ``{cls.replaced_by_cmd.name_and_alias()[0]}`` instead.\n'
f' This command is deprecated. Use ``{cls.get_replaced_command_name()}`` instead.\n'
)

@classmethod
def get_replaced_command_name(cls) -> str:
if isinstance(cls.replaced_by_cmd, tuple):
return ' '.join(cmd.name_and_alias()[0] for cmd in cls.replaced_by_cmd)
return cls.replaced_by_cmd.name_and_alias()[0]


class B2(Command):
"""
Expand Down Expand Up @@ -4670,17 +4676,17 @@ class KeyDeleteSubcommand(KeyDeleteBase):

class ListKeys(CmdReplacedByMixin, KeyListBase):
__doc__ = KeyListBase.__doc__
replaced_by_cmd = Key
replaced_by_cmd = (Key, KeyListSubcommand)


class CreateKey(CmdReplacedByMixin, KeyCreateBase):
__doc__ = KeyCreateBase.__doc__
replaced_by_cmd = Key
replaced_by_cmd = (Key, KeyCreateSubcommand)


class DeleteKey(CmdReplacedByMixin, KeyDeleteBase):
__doc__ = KeyDeleteBase.__doc__
replaced_by_cmd = Key
replaced_by_cmd = (Key, KeyDeleteSubcommand)


class Replication(Command):
Expand Down Expand Up @@ -4734,27 +4740,27 @@ class ReplicationDeleteSubcommand(ReplicationDeleteBase):

class ReplicationSetup(CmdReplacedByMixin, ReplicationSetupBase):
__doc__ = ReplicationSetupBase.__doc__
replaced_by_cmd = Replication
replaced_by_cmd = (Replication, ReplicationSetupSubcommand)


class ReplicationStatus(CmdReplacedByMixin, ReplicationStatusBase):
__doc__ = ReplicationStatusBase.__doc__
replaced_by_cmd = Replication
replaced_by_cmd = (Replication, ReplicationStatusSubcommand)


class ReplicationPause(CmdReplacedByMixin, ReplicationPauseBase):
__doc__ = ReplicationPauseBase.__doc__
replaced_by_cmd = Replication
replaced_by_cmd = (Replication, ReplicationPauseSubcommand)


class ReplicationUnpause(CmdReplacedByMixin, ReplicationUnpauseBase):
__doc__ = ReplicationUnpauseBase.__doc__
replaced_by_cmd = Replication
replaced_by_cmd = (Replication, ReplicationUnpauseSubcommand)


class ReplicationDelete(CmdReplacedByMixin, ReplicationDeleteBase):
__doc__ = ReplicationDeleteBase.__doc__
replaced_by_cmd = Replication
replaced_by_cmd = (Replication, ReplicationDeleteSubcommand)


class Account(Command):
Expand Down Expand Up @@ -4794,17 +4800,17 @@ class AccountClear(AccountClearBase):

class AuthorizeAccount(CmdReplacedByMixin, AccountAuthorizeBase):
__doc__ = AccountAuthorizeBase.__doc__
replaced_by_cmd = Account
replaced_by_cmd = (Account, AccountAuthorize)


class GetAccountInfo(CmdReplacedByMixin, AccountGetBase):
__doc__ = AccountGetBase.__doc__
replaced_by_cmd = Account
replaced_by_cmd = (Account, AccountGet)


class ClearAccount(CmdReplacedByMixin, AccountClearBase):
__doc__ = AccountClearBase.__doc__
replaced_by_cmd = Account
replaced_by_cmd = (Account, AccountClear)


class ConsoleTool:
Expand Down
8 changes: 5 additions & 3 deletions test/integration/test_b2_command_line.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ def test_key_restrictions(b2_tool, bucket_name, sample_file, bucket_factory, b2_
key_two_id, key_two = created_key_two_stdout.split()

create_key_deprecated_pattern = re.compile(
re.escape('WARNING: create-key command is deprecated. Use key instead.')
re.escape('WARNING: `create-key` command is deprecated. Use `key create` instead.')
)
key_three_name = 'clt-testKey-03' + random_hex(6)
created_key_three_stdout = b2_tool.should_succeed(
Expand Down Expand Up @@ -616,7 +616,7 @@ def test_key_restrictions(b2_tool, bucket_name, sample_file, bucket_factory, b2_
b2_tool.should_succeed(['key', 'delete', key_two_id])

delete_key_deprecated_pattern = re.compile(
re.escape('WARNING: delete-key command is deprecated. Use key instead.')
re.escape('WARNING: `delete-key` command is deprecated. Use `key delete` instead.')
)
b2_tool.should_succeed(
['delete-key', key_three_id],
Expand Down Expand Up @@ -2587,7 +2587,9 @@ def test_replication_setup_deprecated(b2_tool, bucket_name, schedule_bucket_clea
def base_test_replication_setup(b2_tool, bucket_name, schedule_bucket_cleanup, use_subcommands):
setup_cmd = ['replication', 'setup'] if use_subcommands else ['replication-setup']
replication_setup_deprecated_pattern = re.compile(
re.escape('WARNING: replication-setup command is deprecated. Use replication instead.')
re.escape(
'WARNING: `replication-setup` command is deprecated. Use `replication setup` instead.'
)
)
replication_setup_expected_stderr_pattern = None if use_subcommands else replication_setup_deprecated_pattern

Expand Down
5 changes: 3 additions & 2 deletions test/unit/console_tool/test_authorize_account.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def test_authorize_with_good_key(b2_cli, b2_cli_is_authorized_afterwards, comman

expected_stderr = "" if len(
command
) == 2 else "WARNING: authorize-account command is deprecated. Use account instead.\n"
) == 2 else "WARNING: `authorize-account` command is deprecated. Use `account authorize` instead.\n"

b2_cli._run_command([*command, b2_cli.account_id, b2_cli.master_key], None, expected_stderr, 0)

Expand Down Expand Up @@ -83,7 +83,8 @@ def test_authorize_using_env_variables(b2_cli):
):
b2_cli._run_command(
["authorize-account"], None,
'WARNING: authorize-account command is deprecated. Use account instead.\n', 0
'WARNING: `authorize-account` command is deprecated. Use `account authorize` instead.\n',
0
)

assert b2_cli.account_info.get_account_auth_token() is not None
Expand Down
8 changes: 4 additions & 4 deletions test/unit/console_tool/test_download_file.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_download_file_by_name(b2_cli, local_file, uploaded_file, tmp_path, flag
output_path=pathlib.Path(output_path).resolve()
),
expected_stderr=
'WARNING: download-file-by-name command is deprecated. Use download-file instead.\n',
'WARNING: `download-file-by-name` command is deprecated. Use `download-file` instead.\n',
)
assert output_path.read_text() == uploaded_file['content']

Expand All @@ -101,7 +101,7 @@ def test_download_file_by_id(b2_cli, uploaded_file, tmp_path, flag, expected_std
['download-file-by-id', flag, '9999', str(output_path)],
expected_stdout=expected_stdout.format(output_path=pathlib.Path(output_path).resolve()),
expected_stderr=
'WARNING: download-file-by-id command is deprecated. Use download-file instead.\n',
'WARNING: `download-file-by-id` command is deprecated. Use `download-file` instead.\n',
)
assert output_path.read_text() == uploaded_file['content']

Expand Down Expand Up @@ -131,7 +131,7 @@ def reader():
output_path=pathlib.Path(output_path).resolve()
),
expected_stderr=
'WARNING: download-file-by-name command is deprecated. Use download-file instead.\n',
'WARNING: `download-file-by-name` command is deprecated. Use `download-file` instead.\n',
)
reader_future.result(timeout=1)
assert output_string == uploaded_file['content']
Expand All @@ -155,7 +155,7 @@ def test_download_file_by_name__to_stdout_by_alias(
b2_cli.run(
['download-file-by-name', '--no-progress', bucket, uploaded_stdout_txt['fileName'], '-'],
expected_stderr=
'WARNING: download-file-by-name command is deprecated. Use download-file instead.\n',
'WARNING: `download-file-by-name` command is deprecated. Use `download-file` instead.\n',
)
assert capfd.readouterr().out == uploaded_stdout_txt['content']
assert not pathlib.Path('-').exists()
Expand Down
2 changes: 1 addition & 1 deletion test/unit/console_tool/test_file_info.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def test_get_file_info(b2_cli, uploaded_file_version):
b2_cli.run(
["get-file-info", uploaded_file_version["fileId"]],
expected_json_in_stdout=uploaded_file_version,
expected_stderr='WARNING: get-file-info command is deprecated. Use file-info instead.\n',
expected_stderr='WARNING: `get-file-info` command is deprecated. Use `file-info` instead.\n',
)


Expand Down
5 changes: 3 additions & 2 deletions test/unit/console_tool/test_get_url.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ def test_make_url(b2_cli, uploaded_file, uploaded_file_url_by_id):
b2_cli.run(
["make-url", uploaded_file["fileId"]],
expected_stdout=f"{uploaded_file_url_by_id}\n",
expected_stderr='WARNING: make-url command is deprecated. Use get-url instead.\n',
expected_stderr='WARNING: `make-url` command is deprecated. Use `get-url` instead.\n',
)


def test_make_friendly_url(b2_cli, bucket, uploaded_file, uploaded_file_url):
b2_cli.run(
["make-friendly-url", bucket, uploaded_file["fileName"]],
expected_stdout=f"{uploaded_file_url}\n",
expected_stderr='WARNING: make-friendly-url command is deprecated. Use get-url instead.\n',
expected_stderr=
'WARNING: `make-friendly-url` command is deprecated. Use `get-url` instead.\n',
)


Expand Down
22 changes: 11 additions & 11 deletions test/unit/test_console_tool.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ def test_create_key_and_authorize_with_it(self):
self._run_command(
['create-key', 'key2', 'listBuckets,listKeys'],
'appKeyId1 appKey1\n',
'WARNING: create-key command is deprecated. Use key instead.\n',
'WARNING: `create-key` command is deprecated. Use `key create` instead.\n',
0,
)

Expand All @@ -374,15 +374,15 @@ def test_create_key_and_authorize_with_it(self):
self._run_command(
['authorize-account', 'appKeyId0', 'appKey0'],
None,
"WARNING: authorize-account command is deprecated. Use account instead.\n",
"WARNING: `authorize-account` command is deprecated. Use `account authorize` instead.\n",
0,
)

# test deprecated command
self._run_command(
['authorize-account', 'appKeyId1', 'appKey1'],
None,
"WARNING: authorize-account command is deprecated. Use account instead.\n",
"WARNING: `authorize-account` command is deprecated. Use `account authorize` instead.\n",
0,
)

Expand Down Expand Up @@ -527,8 +527,8 @@ def test_create_bucket_key_and_authorize_with_it(self):
# test deprecated command
self._run_command(
['create-key', '--bucket', 'my-bucket', 'key2', 'listKeys,listBuckets'],
'appKeyId1 appKey1\n', 'WARNING: create-key command is deprecated. Use key instead.\n',
0
'appKeyId1 appKey1\n',
'WARNING: `create-key` command is deprecated. Use `key create` instead.\n', 0
)

# Authorize with the key
Expand Down Expand Up @@ -599,7 +599,7 @@ def test_deprecated_clear_account(self):
# from the account info.
self._run_command(
['clear-account'], '',
'WARNING: clear-account command is deprecated. Use account instead.\n', 0
'WARNING: `clear-account` command is deprecated. Use `account clear` instead.\n', 0
)
assert self.account_info.get_account_auth_token() is None

Expand Down Expand Up @@ -788,7 +788,7 @@ def test_keys(self):
self._run_command(
['create-key', '--bucket', 'my-bucket-b', 'goodKeyName-Six', capabilities_with_commas],
'appKeyId5 appKey5\n',
'WARNING: create-key command is deprecated. Use key instead.\n',
'WARNING: `create-key` command is deprecated. Use `key create` instead.\n',
0,
)

Expand All @@ -798,7 +798,7 @@ def test_keys(self):
# test deprecated command
self._run_command(
['delete-key', 'appKeyId5'], 'appKeyId5\n',
'WARNING: delete-key command is deprecated. Use key instead.\n', 0
'WARNING: `delete-key` command is deprecated. Use `key delete` instead.\n', 0
)

# Delete one bucket, to test listing when a bucket is gone.
Expand All @@ -824,11 +824,11 @@ def test_keys(self):

self._run_command(
['list-keys'], expected_list_keys_out,
'WARNING: list-keys command is deprecated. Use key instead.\n', 0
'WARNING: `list-keys` command is deprecated. Use `key list` instead.\n', 0
)
self._run_command(
['list-keys', '--long'], expected_list_keys_out_long,
'WARNING: list-keys command is deprecated. Use key instead.\n', 0
'WARNING: `list-keys` command is deprecated. Use `key list` instead.\n', 0
)

# authorize and make calls using application key with no restrictions
Expand Down Expand Up @@ -1683,7 +1683,7 @@ def test_get_account_info(self):
['get-account-info'],
expected_json_in_stdout=expected_json,
expected_stderr=
'WARNING: get-account-info command is deprecated. Use account instead.\n',
'WARNING: `get-account-info` command is deprecated. Use `account get` instead.\n',
)

def test_get_bucket(self):
Expand Down

0 comments on commit 589d49a

Please sign in to comment.