From ef0a5e904bc7bef582b29629956c99deb466de49 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Fri, 2 Jul 2021 17:24:34 +0200 Subject: [PATCH 1/5] feat: different output formats for `bench version` --- frappe/commands/utils.py | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index b944f02af7..7d3aecca1f 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -768,25 +768,45 @@ def set_config(context, key, value, global_=False, parse=False, as_dict=False): @click.command('version') -def get_version(): +@click.option('--output', help='Output format. One of: plain, table, json, legacy.', default='legacy') +def get_version(output): "Show the versions of all the installed apps" from git import Repo - from frappe.utils.change_log import get_app_branch + from frappe.utils.commands import render_table + frappe.init('') + data = [] for app in sorted(frappe.get_all_apps()): - branch_name = get_app_branch(app) module = frappe.get_module(app) app_hooks = frappe.get_module(app + ".hooks") repo = Repo(frappe.get_app_path(app, "..")) - branch = repo.head.ref.name - commit = repo.head.ref.commit.hexsha[:7] - if hasattr(app_hooks, '{0}_version'.format(branch_name)): - click.echo("{0} {1} {2} ({3})".format(app, getattr(app_hooks, '{0}_version'.format(branch_name)), branch, commit)) + app_info = frappe._dict() + app_info.app = app + app_info.branch = repo.head.ref.name + app_info.commit = repo.head.ref.commit.hexsha[:7] + if hasattr(app_hooks, '{0}_version'.format(app_info.branch)): + app_info.version = getattr(app_hooks, '{0}_version'.format(app_info.branch)) elif hasattr(module, "__version__"): - click.echo("{0} {1} {2} ({3})".format(app, module.__version__, branch, commit)) + app_info.version = module.__version__ + + data.append(app_info) + + if output == 'table': + table = [['App', 'Version', 'Branch', 'Commit']] + for app_info in data: + table.append([app_info.app, app_info.version, app_info.branch, app_info.commit]) + render_table(table) + elif output == 'json': + click.echo(json.dumps(data, indent=4)) + elif output == 'plain': + for app_info in data: + click.echo(f'{app_info.app} {app_info.version} {app_info.branch} ({app_info.commit})') + else: # legacy + for app_info in data: + click.echo(f'{app_info.app} {app_info.version}') @click.command('rebuild-global-search') From d1555263e1823f9cd6df626e0c5caa4203c508cf Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 5 Jul 2021 19:52:12 +0200 Subject: [PATCH 2/5] refactor: suggestions from review - use -f / -- format instead of output - get rid of ugly if / else statements --- frappe/commands/utils.py | 46 +++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 7d3aecca1f..1cecdffe1b 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -767,14 +767,15 @@ def set_config(context, key, value, global_=False, parse=False, as_dict=False): frappe.destroy() -@click.command('version') -@click.option('--output', help='Output format. One of: plain, table, json, legacy.', default='legacy') +@click.command("version") +@click.option("-f", "--format", "output", + type=click.Choice(["plain", "table", "json", "legacy"]), help="Output format", default="legacy") def get_version(output): - "Show the versions of all the installed apps" + """Show the versions of all the installed apps.""" from git import Repo from frappe.utils.commands import render_table - frappe.init('') + frappe.init("") data = [] for app in sorted(frappe.get_all_apps()): @@ -786,27 +787,28 @@ def get_version(output): app_info.app = app app_info.branch = repo.head.ref.name app_info.commit = repo.head.ref.commit.hexsha[:7] - - if hasattr(app_hooks, '{0}_version'.format(app_info.branch)): - app_info.version = getattr(app_hooks, '{0}_version'.format(app_info.branch)) - elif hasattr(module, "__version__"): - app_info.version = module.__version__ + app_info.version = getattr(app_hooks, f"{app_info.branch}_version", None) or module.__version__ data.append(app_info) - if output == 'table': - table = [['App', 'Version', 'Branch', 'Commit']] - for app_info in data: - table.append([app_info.app, app_info.version, app_info.branch, app_info.commit]) - render_table(table) - elif output == 'json': - click.echo(json.dumps(data, indent=4)) - elif output == 'plain': - for app_info in data: - click.echo(f'{app_info.app} {app_info.version} {app_info.branch} ({app_info.commit})') - else: # legacy - for app_info in data: - click.echo(f'{app_info.app} {app_info.version}') + { + "legacy": lambda: [ + click.echo(f"{app_info.app} {app_info.version}") + for app_info in data + ], + "plain": lambda: [ + click.echo(f"{app_info.app} {app_info.version} {app_info.branch} ({app_info.commit})") + for app_info in data + ], + "table": lambda: render_table( + [["App", "Version", "Branch", "Commit"]] + + [ + [app_info.app, app_info.version, app_info.branch, app_info.commit] + for app_info in data + ] + ), + "json": lambda: click.echo(json.dumps(data, indent=4)), + }[output]() @click.command('rebuild-global-search') From d7e0479ee742073fc1093ef9f2177153e93ae4a2 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 5 Jul 2021 19:53:02 +0200 Subject: [PATCH 3/5] test: add test for `bench version` --- frappe/tests/test_commands.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 07bdf8791e..8f9bbb1afb 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -426,3 +426,13 @@ class TestCommands(BaseTestCommands): self.assertEqual(self.returncode, 0) self.assertIn("pong", self.stdout) + def test_version(self): + self.execute("bench version") + self.assertEqual(self.returncode, 0) + + for output in ["legacy", "plain", "table", "json"]: + self.execute(f"bench version -f {output}") + self.assertEqual(self.returncode, 0) + + self.execute("bench version -f invalid") + self.assertEqual(self.returncode, 1) From 04094110f5ddb5b2d4c514be43328d9f0e893654 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 13 Jul 2021 13:11:15 +0200 Subject: [PATCH 4/5] fix: handle detached head Workaround for https://github.com/gitpython-developers/GitPython/issues/633 --- frappe/commands/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/commands/utils.py b/frappe/commands/utils.py index 5c6274d348..542f41725b 100644 --- a/frappe/commands/utils.py +++ b/frappe/commands/utils.py @@ -774,6 +774,7 @@ def get_version(output): """Show the versions of all the installed apps.""" from git import Repo from frappe.utils.commands import render_table + from frappe.utils.change_log import get_app_branch frappe.init("") data = [] @@ -785,8 +786,8 @@ def get_version(output): app_info = frappe._dict() app_info.app = app - app_info.branch = repo.head.ref.name - app_info.commit = repo.head.ref.commit.hexsha[:7] + app_info.branch = get_app_branch(app) + app_info.commit = repo.head.object.hexsha[:7] app_info.version = getattr(app_hooks, f"{app_info.branch}_version", None) or module.__version__ data.append(app_info) From fc2c887635168cd18bf18b6eae74fedc58cb9a22 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Tue, 13 Jul 2021 13:23:25 +0200 Subject: [PATCH 5/5] test: fix return code --- frappe/tests/test_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 8f9bbb1afb..54103f0151 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -435,4 +435,4 @@ class TestCommands(BaseTestCommands): self.assertEqual(self.returncode, 0) self.execute("bench version -f invalid") - self.assertEqual(self.returncode, 1) + self.assertEqual(self.returncode, 2)