Skip to content

Commit cb5353b

Browse files
committed
Add SettingOption to bridge Click CLI options to Plain settings
CLI options using envvar= bypass Plain's settings system (no prefix support, invisible to `plain settings list`). SettingOption reads from Plain settings instead, making the setting the single source of truth for defaults and env var resolution.
1 parent cd25849 commit cb5353b

File tree

6 files changed

+264
-65
lines changed

6 files changed

+264
-65
lines changed

plain-jobs/plain/jobs/cli.py

Lines changed: 58 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import click
99

10-
from plain.cli import register_cli
10+
from plain.cli import SettingOption, register_cli
1111
from plain.runtime import settings
1212
from plain.utils import timezone
1313

@@ -37,30 +37,30 @@ def cli() -> None:
3737
@click.option(
3838
"--max-processes",
3939
"max_processes",
40-
default=None,
4140
type=int,
42-
envvar="PLAIN_JOBS_WORKER_MAX_PROCESSES",
41+
cls=SettingOption,
42+
setting="JOBS_WORKER_MAX_PROCESSES",
4343
)
4444
@click.option(
4545
"--max-jobs-per-process",
4646
"max_jobs_per_process",
47-
default=None,
4847
type=int,
49-
envvar="PLAIN_JOBS_WORKER_MAX_JOBS_PER_PROCESS",
48+
cls=SettingOption,
49+
setting="JOBS_WORKER_MAX_JOBS_PER_PROCESS",
5050
)
5151
@click.option(
5252
"--max-pending-per-process",
5353
"max_pending_per_process",
54-
default=10,
5554
type=int,
56-
envvar="PLAIN_JOBS_WORKER_MAX_PENDING_PER_PROCESS",
55+
cls=SettingOption,
56+
setting="JOBS_WORKER_MAX_PENDING_PER_PROCESS",
5757
)
5858
@click.option(
5959
"--stats-every",
6060
"stats_every",
61-
default=60,
6261
type=int,
63-
envvar="PLAIN_JOBS_WORKER_STATS_EVERY",
62+
cls=SettingOption,
63+
setting="JOBS_WORKER_STATS_EVERY",
6464
)
6565
@click.option(
6666
"--reload",
@@ -78,64 +78,59 @@ def worker(
7878
"""Run the job worker"""
7979
jobs_schedule = load_schedule(settings.JOBS_SCHEDULE)
8080

81-
if reload:
82-
from plain.internal.reloader import Reloader
83-
84-
# Track whether we should continue restarting
85-
should_restart = {"value": True}
86-
current_worker = {"instance": None}
87-
88-
def file_changed(filename: str) -> None:
89-
if current_worker["instance"]:
90-
current_worker["instance"].shutdown()
91-
92-
def signal_shutdown(signalnum: int, _: Any) -> None:
93-
should_restart["value"] = False
94-
if current_worker["instance"]:
95-
current_worker["instance"].shutdown()
96-
97-
# Allow the worker to be stopped gracefully on SIGTERM/SIGINT
98-
signal.signal(signal.SIGTERM, signal_shutdown)
99-
signal.signal(signal.SIGINT, signal_shutdown)
100-
101-
# Start file watcher once, outside the loop
102-
reloader = Reloader(callback=file_changed, watch_html=False)
103-
reloader.start()
104-
105-
while should_restart["value"]:
106-
worker = Worker(
107-
queues=list(queues),
108-
jobs_schedule=jobs_schedule,
109-
max_processes=max_processes,
110-
max_jobs_per_process=max_jobs_per_process,
111-
max_pending_per_process=max_pending_per_process,
112-
stats_every=stats_every,
113-
)
114-
current_worker["instance"] = worker
115-
116-
# Start processing jobs (blocks until shutdown)
117-
worker.run()
81+
worker_kwargs = {
82+
"queues": list(queues),
83+
"jobs_schedule": jobs_schedule,
84+
"max_processes": max_processes,
85+
"max_jobs_per_process": max_jobs_per_process,
86+
"max_pending_per_process": max_pending_per_process,
87+
"stats_every": stats_every,
88+
}
11889

90+
if reload:
91+
_run_with_reload(worker_kwargs)
11992
else:
120-
worker = Worker(
121-
queues=list(queues),
122-
jobs_schedule=jobs_schedule,
123-
max_processes=max_processes,
124-
max_jobs_per_process=max_jobs_per_process,
125-
max_pending_per_process=max_pending_per_process,
126-
stats_every=stats_every,
127-
)
93+
_run_once(worker_kwargs)
94+
95+
96+
def _run_with_reload(worker_kwargs: dict[str, Any]) -> None:
97+
from plain.internal.reloader import Reloader
98+
99+
should_restart = {"value": True}
100+
current_worker: dict[str, Worker | None] = {"instance": None}
101+
102+
def file_changed(filename: str) -> None:
103+
if current_worker["instance"]:
104+
current_worker["instance"].shutdown()
105+
106+
def signal_shutdown(signalnum: int, _: Any) -> None:
107+
should_restart["value"] = False
108+
if current_worker["instance"]:
109+
current_worker["instance"].shutdown()
110+
111+
signal.signal(signal.SIGTERM, signal_shutdown)
112+
signal.signal(signal.SIGINT, signal_shutdown)
113+
114+
reloader = Reloader(callback=file_changed, watch_html=False)
115+
reloader.start()
116+
117+
while should_restart["value"]:
118+
w = Worker(**worker_kwargs)
119+
current_worker["instance"] = w
120+
w.run()
121+
122+
123+
def _run_once(worker_kwargs: dict[str, Any]) -> None:
124+
w = Worker(**worker_kwargs)
128125

129-
def _shutdown(signalnum: int, _: Any) -> None:
130-
logger.info("Job worker shutdown signal received signalnum=%s", signalnum)
131-
worker.shutdown()
126+
def _shutdown(signalnum: int, _: Any) -> None:
127+
logger.info("Job worker shutdown signal received signalnum=%s", signalnum)
128+
w.shutdown()
132129

133-
# Allow the worker to be stopped gracefully on SIGTERM
134-
signal.signal(signal.SIGTERM, _shutdown)
135-
signal.signal(signal.SIGINT, _shutdown)
130+
signal.signal(signal.SIGTERM, _shutdown)
131+
signal.signal(signal.SIGINT, _shutdown)
136132

137-
# Start processing jobs
138-
worker.run()
133+
w.run()
139134

140135

141136
@cli.command()
@@ -195,8 +190,7 @@ def run(job_class_name: str) -> None:
195190
def list_jobs() -> None:
196191
"""List all registered jobs"""
197192
for name, job_class in jobs_registry.jobs.items():
198-
click.secho(f"{name}", bold=True, nl=False)
199-
# Get description from class docstring
193+
click.secho(name, bold=True, nl=False)
200194
description = job_class.__doc__.strip() if job_class.__doc__ else ""
201195
if description:
202196
click.secho(f": {description}", dim=True)

plain-jobs/plain/jobs/default_settings.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,7 @@
44
"plain.jobs.middleware.AppLoggerMiddleware",
55
]
66
JOBS_SCHEDULE: list[tuple[str, str]] = []
7+
JOBS_WORKER_MAX_PROCESSES: int | None = None
8+
JOBS_WORKER_MAX_JOBS_PER_PROCESS: int | None = None
9+
JOBS_WORKER_MAX_PENDING_PER_PROCESS: int = 10
10+
JOBS_WORKER_STATS_EVERY: int = 60

plain/plain/cli/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1+
from .options import SettingOption
12
from .registry import register_cli
23
from .runtime import common_command
34

4-
__all__ = ["register_cli", "common_command"]
5+
__all__ = ["SettingOption", "register_cli", "common_command"]

plain/plain/cli/options.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
from __future__ import annotations
2+
3+
from typing import Any
4+
5+
import click
6+
7+
8+
class SettingOption(click.Option):
9+
"""A Click option that reads its value from Plain settings instead of os.environ.
10+
11+
Usage:
12+
@click.option("--max-processes", cls=SettingOption, setting="JOBS_WORKER_MAX_PROCESSES")
13+
14+
CLI args always take priority. The setting's value (including package
15+
defaults) is the single source of truth — no need for Click `default=`.
16+
"""
17+
18+
def __init__(self, *args: Any, setting: str | None = None, **kwargs: Any) -> None:
19+
if setting and kwargs.get("envvar"):
20+
raise ValueError(
21+
"Cannot use both 'setting' and 'envvar' on the same option. "
22+
"Use 'setting' to read from Plain settings."
23+
)
24+
if setting and "default" in kwargs:
25+
raise ValueError(
26+
"Cannot use both 'setting' and 'default' on the same option. "
27+
"The setting's default value is the single source of truth."
28+
)
29+
self.setting_name = setting
30+
if setting:
31+
kwargs.setdefault("show_default", True)
32+
super().__init__(*args, **kwargs)
33+
34+
def get_default(self, ctx: click.Context, call: bool = True) -> Any:
35+
if self.setting_name:
36+
try:
37+
from plain.runtime import settings
38+
39+
settings._setup()
40+
defn = settings._settings.get(self.setting_name)
41+
if defn is not None:
42+
return defn.value
43+
except Exception:
44+
if ctx.resilient_parsing:
45+
return None
46+
raise
47+
return super().get_default(ctx, call=call)
48+
49+
def get_help_record(self, ctx: click.Context) -> tuple[str, str] | None:
50+
result = super().get_help_record(ctx)
51+
if result and self.setting_name:
52+
name, help_text = result
53+
setting_str = f"setting: {self.setting_name}"
54+
if help_text and help_text.rstrip().endswith("]"):
55+
bracket_start = help_text.rindex("[")
56+
inside = help_text[bracket_start + 1 : -1]
57+
help_text = f"{help_text[:bracket_start]}[{setting_str}; {inside}]"
58+
elif help_text:
59+
help_text = f"{help_text} [{setting_str}]"
60+
else:
61+
help_text = f"[{setting_str}]"
62+
return name, help_text
63+
return result

plain/tests/app/test/default_settings.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@
22
EXPLICIT_SETTING: str = "unchanged explicit"
33
ENV_SETTING: int = 0
44
ENV_OVERRIDDEN_SETTING: str = "unchanged env overridden"
5+
NULLABLE_SETTING: int | None = None

plain/tests/test_setting_option.py

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
from __future__ import annotations
2+
3+
import click
4+
import pytest
5+
from click.testing import CliRunner
6+
7+
from plain.cli.options import SettingOption
8+
9+
10+
@click.command()
11+
@click.option(
12+
"--value",
13+
type=int,
14+
cls=SettingOption,
15+
setting="ENV_SETTING",
16+
)
17+
def sample_cmd(value):
18+
click.echo(f"value={value}")
19+
20+
21+
def test_cli_arg_overrides_setting():
22+
runner = CliRunner()
23+
# ENV_SETTING is set to 1 via conftest.py env var
24+
result = runner.invoke(sample_cmd, ["--value", "42"])
25+
assert result.exit_code == 0
26+
assert "value=42" in result.output
27+
28+
29+
def test_setting_from_env_var():
30+
runner = CliRunner()
31+
# ENV_SETTING = 1 (set via PLAIN_ENV_SETTING in conftest.py)
32+
result = runner.invoke(sample_cmd, [])
33+
assert result.exit_code == 0
34+
assert "value=1" in result.output
35+
36+
37+
def test_none_when_setting_not_found():
38+
@click.command()
39+
@click.option(
40+
"--value",
41+
type=int,
42+
cls=SettingOption,
43+
setting="NONEXISTENT_SETTING_XYZ",
44+
)
45+
def cmd(value):
46+
click.echo(f"value={value}")
47+
48+
runner = CliRunner()
49+
result = runner.invoke(cmd, [])
50+
assert result.exit_code == 0
51+
assert "value=None" in result.output
52+
53+
54+
def test_setting_from_explicit_value():
55+
@click.command()
56+
@click.option(
57+
"--value",
58+
type=str,
59+
cls=SettingOption,
60+
setting="EXPLICIT_SETTING",
61+
)
62+
def cmd(value):
63+
click.echo(f"value={value}")
64+
65+
runner = CliRunner()
66+
result = runner.invoke(cmd, [])
67+
assert result.exit_code == 0
68+
assert "value=explicitly changed" in result.output
69+
70+
71+
def test_raises_if_both_setting_and_envvar():
72+
with pytest.raises(ValueError, match="Cannot use both"):
73+
74+
@click.command()
75+
@click.option(
76+
"--value",
77+
type=int,
78+
cls=SettingOption,
79+
setting="SOME_SETTING",
80+
envvar="SOME_ENV_VAR",
81+
)
82+
def cmd(value):
83+
pass
84+
85+
86+
def test_raises_if_both_setting_and_default():
87+
with pytest.raises(ValueError, match="Cannot use both"):
88+
89+
@click.command()
90+
@click.option(
91+
"--value",
92+
type=int,
93+
cls=SettingOption,
94+
setting="SOME_SETTING",
95+
default=42,
96+
)
97+
def cmd(value):
98+
pass
99+
100+
101+
def test_none_valued_setting():
102+
"""A setting that exists but has None as its value passes through as None."""
103+
104+
@click.command()
105+
@click.option(
106+
"--value",
107+
type=int,
108+
cls=SettingOption,
109+
setting="NULLABLE_SETTING",
110+
)
111+
def cmd(value):
112+
click.echo(f"value={value}")
113+
114+
runner = CliRunner()
115+
result = runner.invoke(cmd, [])
116+
assert result.exit_code == 0
117+
assert "value=None" in result.output
118+
119+
120+
def test_package_default_is_used():
121+
"""Package defaults flow through SettingOption as the single source of truth."""
122+
123+
@click.command()
124+
@click.option(
125+
"--value",
126+
type=str,
127+
cls=SettingOption,
128+
setting="DEFAULT_SETTING",
129+
)
130+
def cmd(value):
131+
click.echo(f"value={value}")
132+
133+
runner = CliRunner()
134+
result = runner.invoke(cmd, [])
135+
assert result.exit_code == 0
136+
assert "value=unchanged default" in result.output

0 commit comments

Comments
 (0)