From 6b84c9ccf5fc3c389223f37cf4fb1865be430448 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 14:38:50 +0530 Subject: [PATCH 1/6] feat: Check scheduler status via CLI new: `bench --site scheduler status` --- frappe/commands/scheduler.py | 38 +++++++++++++++--------------------- frappe/utils/scheduler.py | 8 +++++--- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/frappe/commands/scheduler.py b/frappe/commands/scheduler.py index a6610c9213..7ced1e4d11 100755 --- a/frappe/commands/scheduler.py +++ b/frappe/commands/scheduler.py @@ -74,37 +74,31 @@ def disable_scheduler(context): @click.command("scheduler") @click.option("--site", help="site name") -@click.argument("state", type=click.Choice(["pause", "resume", "disable", "enable"])) +@click.argument("state", type=click.Choice(["pause", "resume", "disable", "enable", "status"])) @pass_context -def scheduler(context, state, site=None): +def scheduler(context, state: str, site: str | None = None): """Control scheduler state.""" + import frappe import frappe.utils.scheduler from frappe.installer import update_site_config - if not site: - site = get_site(context) + site = site or get_site(context) - try: - frappe.init(site=site) - - if state == "pause": - update_site_config("pause_scheduler", 1) - elif state == "resume": - update_site_config("pause_scheduler", 0) - elif state == "disable": - frappe.connect() - frappe.utils.scheduler.disable_scheduler() - frappe.db.commit() - elif state == "enable": - frappe.connect() - frappe.utils.scheduler.enable_scheduler() - frappe.db.commit() + with frappe.init_site(site=site): + match state: + case "status": + frappe.connect() + status = "disabled" if frappe.utils.scheduler.is_scheduler_inactive(verbose=verbose) else "enabled" + return print(output[format].format(status=status, site=site)) + case "pause" | "resume": + update_site_config("pause_scheduler", state == "pause") + case "enable" | "disable": + frappe.connect() + frappe.utils.scheduler.toggle_scheduler(state == "enable") + frappe.db.commit() print(f"Scheduler {state}d for site {site}") - finally: - frappe.destroy() - @click.command("set-maintenance-mode") @click.option("--site", help="site name") diff --git a/frappe/utils/scheduler.py b/frappe/utils/scheduler.py index 03c1b37a43..146a7fa244 100755 --- a/frappe/utils/scheduler.py +++ b/frappe/utils/scheduler.py @@ -107,16 +107,18 @@ def is_scheduler_inactive() -> bool: return False -def is_scheduler_disabled() -> bool: +def is_scheduler_disabled(verbose=True) -> bool: if frappe.conf.disable_scheduler: - cprint(f"{frappe.local.site}: frappe.conf.disable_scheduler is SET") + if verbose: + cprint(f"{frappe.local.site}: frappe.conf.disable_scheduler is SET") return True scheduler_disabled = not frappe.utils.cint( frappe.db.get_single_value("System Settings", "enable_scheduler") ) if scheduler_disabled: - cprint(f"{frappe.local.site}: SystemSettings.enable_scheduler is UNSET") + if verbose: + cprint(f"{frappe.local.site}: SystemSettings.enable_scheduler is UNSET") return scheduler_disabled From 4738a1422da05e714f201368a90f936a0d743ee2 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 14:39:16 +0530 Subject: [PATCH 2/6] fix: Add format, verbose options to scheduler --- frappe/commands/scheduler.py | 17 ++++++++++++++--- frappe/utils/scheduler.py | 10 ++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/frappe/commands/scheduler.py b/frappe/commands/scheduler.py index 7ced1e4d11..6365ecdd26 100755 --- a/frappe/commands/scheduler.py +++ b/frappe/commands/scheduler.py @@ -75,8 +75,12 @@ def disable_scheduler(context): @click.command("scheduler") @click.option("--site", help="site name") @click.argument("state", type=click.Choice(["pause", "resume", "disable", "enable", "status"])) +@click.option( + "--format", "-f", default="text", type=click.Choice(["json", "text"]), help="Output format" +) +@click.option("--verbose", "-v", is_flag=True, help="Verbose output") @pass_context -def scheduler(context, state: str, site: str | None = None): +def scheduler(context, state: str, format: str, verbose: bool = False, site: str | None = None): """Control scheduler state.""" import frappe import frappe.utils.scheduler @@ -84,11 +88,18 @@ def scheduler(context, state: str, site: str | None = None): site = site or get_site(context) + output = { + "text": "Scheduler is {status} for site {site}", + "json": '{{"status": "{status}", "site": "{site}"}}', + } + with frappe.init_site(site=site): match state: case "status": frappe.connect() - status = "disabled" if frappe.utils.scheduler.is_scheduler_inactive(verbose=verbose) else "enabled" + status = ( + "disabled" if frappe.utils.scheduler.is_scheduler_inactive(verbose=verbose) else "enabled" + ) return print(output[format].format(status=status, site=site)) case "pause" | "resume": update_site_config("pause_scheduler", state == "pause") @@ -97,7 +108,7 @@ def scheduler(context, state: str, site: str | None = None): frappe.utils.scheduler.toggle_scheduler(state == "enable") frappe.db.commit() - print(f"Scheduler {state}d for site {site}") + print(output[format].format(status=f"{state}d", site=site)) @click.command("set-maintenance-mode") diff --git a/frappe/utils/scheduler.py b/frappe/utils/scheduler.py index 146a7fa244..8cda71ee9a 100755 --- a/frappe/utils/scheduler.py +++ b/frappe/utils/scheduler.py @@ -92,16 +92,18 @@ def enqueue_events(site: str) -> list[str] | None: return enqueued_jobs -def is_scheduler_inactive() -> bool: +def is_scheduler_inactive(verbose=True) -> bool: if frappe.local.conf.maintenance_mode: - cprint(f"{frappe.local.site}: Maintenance mode is ON") + if verbose: + cprint(f"{frappe.local.site}: Maintenance mode is ON") return True if frappe.local.conf.pause_scheduler: - cprint(f"{frappe.local.site}: frappe.conf.pause_scheduler is SET") + if verbose: + cprint(f"{frappe.local.site}: frappe.conf.pause_scheduler is SET") return True - if is_scheduler_disabled(): + if is_scheduler_disabled(verbose=verbose): return True return False From 18eabf515312cda03df9c12188c471ea9c301385 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 14:39:39 +0530 Subject: [PATCH 3/6] fix(dx): Don't reload web workers if tests are changed in dev server --- frappe/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/app.py b/frappe/app.py index 2fe9991c4c..03414646ef 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -376,6 +376,7 @@ def serve( "0.0.0.0", int(port), application, + exclude_patterns=["test_*"], use_reloader=False if in_test_env else not no_reload, use_debugger=not in_test_env, use_evalex=not in_test_env, From 32cf13cb29a9b38fd9302de33ebab6e4000a2134 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Fri, 10 Feb 2023 14:45:39 +0530 Subject: [PATCH 4/6] chore: Cleanup imports --- frappe/commands/scheduler.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/frappe/commands/scheduler.py b/frappe/commands/scheduler.py index 6365ecdd26..36fa81f8a5 100755 --- a/frappe/commands/scheduler.py +++ b/frappe/commands/scheduler.py @@ -5,7 +5,6 @@ import click import frappe from frappe.commands import get_site, pass_context from frappe.exceptions import SiteNotSpecifiedError -from frappe.utils import cint @click.command("trigger-scheduler-event", help="Trigger a scheduler event") @@ -83,8 +82,7 @@ def disable_scheduler(context): def scheduler(context, state: str, format: str, verbose: bool = False, site: str | None = None): """Control scheduler state.""" import frappe - import frappe.utils.scheduler - from frappe.installer import update_site_config + from frappe.utils.scheduler import is_scheduler_inactive, toggle_scheduler site = site or get_site(context) @@ -97,15 +95,15 @@ def scheduler(context, state: str, format: str, verbose: bool = False, site: str match state: case "status": frappe.connect() - status = ( - "disabled" if frappe.utils.scheduler.is_scheduler_inactive(verbose=verbose) else "enabled" - ) + status = "disabled" if is_scheduler_inactive(verbose=verbose) else "enabled" return print(output[format].format(status=status, site=site)) case "pause" | "resume": + from frappe.installer import update_site_config + update_site_config("pause_scheduler", state == "pause") case "enable" | "disable": frappe.connect() - frappe.utils.scheduler.toggle_scheduler(state == "enable") + toggle_scheduler(state == "enable") frappe.db.commit() print(output[format].format(status=f"{state}d", site=site)) From c337e532d88b85c1f9ccde5e73bf7fea9d594404 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 13 Feb 2023 14:55:23 +0530 Subject: [PATCH 5/6] test: Add test for db-console pass extra params to client Added test for https://github.com/frappe/frappe/pull/19809 --- frappe/tests/test_commands.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 4a10484d1d..66f797d168 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -773,3 +773,9 @@ class TestDBCli(BaseTestCommands): def test_db_cli(self): self.execute("bench --site {site} db-console", kwargs={"cmd_input": rb"\q"}) self.assertEqual(self.returncode, 0) + + @run_only_if(db_type_is.MARIADB) + def test_db_cli_with_sql(self): + self.execute("bench --site {site} db-console -e 'select 1'") + self.assertEqual(self.returncode, 0) + self.assertIn("1", self.stdout) From 2c348af4f7a9b2332080d5b4556666a5ccbd6e36 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Mon, 13 Feb 2023 15:08:56 +0530 Subject: [PATCH 6/6] test: Add tests for scheduler cli --- frappe/tests/test_commands.py | 44 +++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py index 66f797d168..7bda577bb6 100644 --- a/frappe/tests/test_commands.py +++ b/frappe/tests/test_commands.py @@ -33,6 +33,7 @@ from frappe.tests.utils import FrappeTestCase, timeout from frappe.utils import add_to_date, get_bench_path, get_bench_relative_path, now from frappe.utils.backups import BackupGenerator, fetch_latest_backups from frappe.utils.jinja_globals import bundled_asset +from frappe.utils.scheduler import enable_scheduler, is_scheduler_inactive _result: Result | None = None TEST_SITE = "commands-site-O4PN2QKA.test" # added random string tag to avoid collisions @@ -779,3 +780,46 @@ class TestDBCli(BaseTestCommands): self.execute("bench --site {site} db-console -e 'select 1'") self.assertEqual(self.returncode, 0) self.assertIn("1", self.stdout) + + +class TestSchedulerCLI(BaseTestCommands): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.is_scheduler_active = not is_scheduler_inactive() + + @classmethod + def tearDownClass(cls): + super().tearDownClass() + if cls.is_scheduler_active: + enable_scheduler() + + def test_scheduler_status(self): + self.execute("bench --site {site} scheduler status") + self.assertEqual(self.returncode, 0) + self.assertRegex(self.stdout, r"Scheduler is (disabled|enabled) for site .*") + + self.execute("bench --site {site} scheduler status -f json") + parsed_output = frappe.parse_json(self.stdout) + self.assertEqual(self.returncode, 0) + self.assertIsInstance(parsed_output, dict) + self.assertIn("status", parsed_output) + self.assertIn("site", parsed_output) + + def test_scheduler_enable_disable(self): + self.execute("bench --site {site} scheduler disable") + self.assertEqual(self.returncode, 0) + self.assertRegex(self.stdout, r"Scheduler is disabled for site .*") + + self.execute("bench --site {site} scheduler enable") + self.assertEqual(self.returncode, 0) + self.assertRegex(self.stdout, r"Scheduler is enabled for site .*") + + def test_scheduler_pause_resume(self): + self.execute("bench --site {site} scheduler pause") + self.assertEqual(self.returncode, 0) + self.assertRegex(self.stdout, r"Scheduler is paused for site .*") + + self.execute("bench --site {site} scheduler resume") + self.assertEqual(self.returncode, 0) + self.assertRegex(self.stdout, r"Scheduler is resumed for site .*")