Studio: accept --not-secure as a back-compat alias for --no-secure (#6568)
* Studio: accept --not-secure as a back-compat alias for --no-secure PR #6560 renamed the negative secure flag from --not-secure to --no-secure to match argparse.BooleanOptionalAction. Re-add --not-secure as a hidden, deprecated alias at both CLI layers so existing scripts and muscle memory keep working, while --no-secure stays the documented spelling. - studio/backend/run.py: extract the CLI parser into _build_arg_parser() so the flag wiring is unit-testable, and register --not-secure as a hidden store_false alias for --no-secure. Last flag wins, matching BooleanOptionalAction semantics. - unsloth_cli/commands/studio.py: add a hidden --not-secure option to `unsloth studio` and `unsloth studio run`; it forces secure off and forwards the canonical --no-secure to the backend. - Tests at both layers for the alias and its polarity. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: address review on --not-secure alias - run.py: use argparse.SUPPRESS for the --not-secure default so the alias never contributes a namespace default (the canonical --secure owns it). - studio.py: resolve --not-secure last-wins from argv via _resolve_secure() so `--not-secure --secure` keeps secure on, matching the backend's BooleanOptionalAction and how --secure/--no-secure already behave. - Add a CLI last-wins test covering both flag orders. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
2ec0b88471
commit
6254ab37c3
4 changed files with 125 additions and 16 deletions
|
|
@ -1174,18 +1174,20 @@ def run_server(
|
|||
return app
|
||||
|
||||
|
||||
# For direct execution (also invoked by CLI via os.execvp / subprocess).
|
||||
if __name__ == "__main__":
|
||||
import argparse
|
||||
import signal
|
||||
import traceback
|
||||
# Mirror unsloth_cli/commands/studio.py's _PARALLEL_*. Default 1 is for direct
|
||||
# backend launches; `unsloth studio run` always passes its own value (4).
|
||||
_PARALLEL_MIN = 1
|
||||
_PARALLEL_MAX = 64
|
||||
_PARALLEL_DEFAULT_PLAIN = 1
|
||||
|
||||
# Ensure stderr handles Unicode on Windows (non-ASCII path tracebacks).
|
||||
if sys.platform == "win32" and hasattr(sys.stderr, "reconfigure"):
|
||||
try:
|
||||
sys.stderr.reconfigure(encoding = "utf-8", errors = "replace")
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
def _build_arg_parser():
|
||||
"""Build the backend CLI argument parser.
|
||||
|
||||
Extracted from the __main__ block so the flag wiring (notably the
|
||||
--secure/--no-secure polarity and its --not-secure alias) stays unit-testable.
|
||||
"""
|
||||
import argparse
|
||||
|
||||
parser = argparse.ArgumentParser(description = "Run Unsloth UI Backend server")
|
||||
parser.add_argument(
|
||||
|
|
@ -1221,6 +1223,14 @@ if __name__ == "__main__":
|
|||
"if the tunnel can't start. Without it, --no-secure also serves the raw "
|
||||
"0.0.0.0 port, which is reachable from anywhere on the network",
|
||||
)
|
||||
# Back-compat: accept --not-secure as a hidden alias for --no-secure.
|
||||
parser.add_argument(
|
||||
"--not-secure",
|
||||
dest = "secure",
|
||||
action = "store_false",
|
||||
default = argparse.SUPPRESS,
|
||||
help = argparse.SUPPRESS,
|
||||
)
|
||||
# Tri-state tool policy: no flag -> None (tools on, per-request honored);
|
||||
# --enable-tools/--disable-tools force on/off.
|
||||
parser.add_argument(
|
||||
|
|
@ -1238,11 +1248,6 @@ if __name__ == "__main__":
|
|||
default = None,
|
||||
help = "Force server-side tools off for every request.",
|
||||
)
|
||||
# Mirror unsloth_cli/commands/studio.py's _PARALLEL_*. Default 1 is for direct
|
||||
# backend launches; `unsloth studio run` always passes its own value (4).
|
||||
_PARALLEL_MIN = 1
|
||||
_PARALLEL_MAX = 64
|
||||
_PARALLEL_DEFAULT_PLAIN = 1
|
||||
parser.add_argument(
|
||||
"--parallel",
|
||||
"--n-parallel",
|
||||
|
|
@ -1253,7 +1258,22 @@ if __name__ == "__main__":
|
|||
f"Default {_PARALLEL_DEFAULT_PLAIN}; `unsloth studio run` uses 4."
|
||||
),
|
||||
)
|
||||
return parser
|
||||
|
||||
|
||||
# For direct execution (also invoked by CLI via os.execvp / subprocess).
|
||||
if __name__ == "__main__":
|
||||
import signal
|
||||
import traceback
|
||||
|
||||
# Ensure stderr handles Unicode on Windows (non-ASCII path tracebacks).
|
||||
if sys.platform == "win32" and hasattr(sys.stderr, "reconfigure"):
|
||||
try:
|
||||
sys.stderr.reconfigure(encoding = "utf-8", errors = "replace")
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
parser = _build_arg_parser()
|
||||
args = parser.parse_args()
|
||||
if not _PARALLEL_MIN <= args.parallel <= _PARALLEL_MAX:
|
||||
parser.error(f"--parallel must be between {_PARALLEL_MIN} and {_PARALLEL_MAX}")
|
||||
|
|
|
|||
|
|
@ -60,6 +60,20 @@ def test_run_server_accepts_secure_kwarg():
|
|||
assert inspect.signature(run.run_server).parameters["secure"].default is False
|
||||
|
||||
|
||||
def test_arg_parser_secure_polarity_and_not_secure_alias():
|
||||
# --secure/--no-secure is the documented flag; --not-secure is a hidden,
|
||||
# back-compat alias for --no-secure. Last flag wins (BooleanOptionalAction).
|
||||
import run
|
||||
|
||||
parser = run._build_arg_parser()
|
||||
assert parser.parse_args([]).secure is False
|
||||
assert parser.parse_args(["--secure"]).secure is True
|
||||
assert parser.parse_args(["--no-secure"]).secure is False
|
||||
assert parser.parse_args(["--not-secure"]).secure is False
|
||||
assert parser.parse_args(["--secure", "--not-secure"]).secure is False
|
||||
assert parser.parse_args(["--not-secure", "--secure"]).secure is True
|
||||
|
||||
|
||||
def test_run_server_accepts_enable_tools_kwarg():
|
||||
import inspect
|
||||
|
||||
|
|
|
|||
|
|
@ -260,6 +260,28 @@ _PARALLEL_DEFAULT_RUN = 4 # pre-PR hardcoded for `unsloth studio run`
|
|||
_PARALLEL_DEFAULT_PLAIN = 1 # pre-PR effective for plain `unsloth studio`
|
||||
|
||||
|
||||
def _resolve_secure(secure: bool, not_secure: bool) -> bool:
|
||||
"""Reconcile the deprecated --not-secure alias with --secure/--no-secure.
|
||||
|
||||
Typer parses --secure and --not-secure as independent options, so the alias
|
||||
cannot lean on Click's last-wins ordering the way --secure/--no-secure do.
|
||||
Restore that ordering from argv: --not-secure only forces secure off when it
|
||||
is the last of the secure flags on the command line, matching the backend's
|
||||
BooleanOptionalAction.
|
||||
"""
|
||||
if not not_secure:
|
||||
return secure
|
||||
last_secure = max(
|
||||
(i for i, a in enumerate(sys.argv) if a in ("--secure", "--no-secure")),
|
||||
default = -1,
|
||||
)
|
||||
last_not_secure = max(
|
||||
(i for i, a in enumerate(sys.argv) if a == "--not-secure"),
|
||||
default = -1,
|
||||
)
|
||||
return secure if last_secure > last_not_secure else False
|
||||
|
||||
|
||||
def _iter_editable_studio_source_roots(venv_dir: Path):
|
||||
"""Yield repo roots from setuptools `__editable___*_finder.py` files in
|
||||
*venv_dir*'s site-packages whose MAPPING includes a `studio` entry.
|
||||
|
|
@ -675,6 +697,12 @@ def studio_default(
|
|||
"if the tunnel can't start. Without it, --no-secure also serves the raw "
|
||||
"0.0.0.0 port, which is reachable from anywhere on the network.",
|
||||
),
|
||||
not_secure: bool = typer.Option(
|
||||
False,
|
||||
"--not-secure",
|
||||
hidden = True,
|
||||
help = "Deprecated alias for --no-secure.",
|
||||
),
|
||||
verbose: bool = typer.Option(
|
||||
False,
|
||||
"--verbose",
|
||||
|
|
@ -690,6 +718,8 @@ def studio_default(
|
|||
),
|
||||
):
|
||||
"""Launch the Unsloth Studio server."""
|
||||
# Back-compat: --not-secure is a deprecated alias for --no-secure.
|
||||
secure = _resolve_secure(secure, not_secure)
|
||||
# Runs before every subcommand (run/setup/update/...).
|
||||
_ensure_studio_env_exported()
|
||||
if ctx.invoked_subcommand is not None:
|
||||
|
|
@ -1039,6 +1069,12 @@ def run(
|
|||
"if the tunnel can't start. Without it, --no-secure also serves the raw "
|
||||
"0.0.0.0 port, which is reachable from anywhere on the network.",
|
||||
),
|
||||
not_secure: bool = typer.Option(
|
||||
False,
|
||||
"--not-secure",
|
||||
hidden = True,
|
||||
help = "Deprecated alias for --no-secure.",
|
||||
),
|
||||
tensor_parallel: bool = typer.Option(
|
||||
False,
|
||||
"--tensor-parallel/--no-tensor-parallel",
|
||||
|
|
@ -1066,6 +1102,8 @@ def run(
|
|||
unsloth studio run --model some-model --chat-template-file /path/to/tpl.jinja
|
||||
unsloth studio run --model unsloth/Qwen3-27B-GGUF --gguf-variant Q8_0 --tensor-parallel
|
||||
"""
|
||||
# Back-compat: --not-secure is a deprecated alias for --no-secure.
|
||||
secure = _resolve_secure(secure, not_secure)
|
||||
extra_llama_args: List[str] = list(ctx.args) if ctx.args else []
|
||||
|
||||
# Set before any re-exec so the in-venv server inherits it via the env.
|
||||
|
|
|
|||
|
|
@ -48,6 +48,17 @@ def test_studio_default_exposes_secure_option_default_off():
|
|||
assert getattr(opt, "default", None) is False
|
||||
|
||||
|
||||
def test_secure_exposes_hidden_not_secure_alias():
|
||||
# --not-secure is a hidden, deprecated alias for --no-secure on both commands.
|
||||
import inspect
|
||||
for fn in (_studio().run, _studio().studio_default):
|
||||
opt = inspect.signature(fn).parameters["not_secure"].default
|
||||
decls = set(getattr(opt, "param_decls", []) or [])
|
||||
assert "--not-secure" in decls
|
||||
assert getattr(opt, "hidden", False) is True
|
||||
assert getattr(opt, "default", None) is False
|
||||
|
||||
|
||||
# ── re-exec capture plumbing (mirrors test_studio_cloudflare_flag.py) ─
|
||||
|
||||
|
||||
|
|
@ -132,6 +143,7 @@ def _invoke_studio_default(monkeypatch, args):
|
|||
(None, "--no-secure", "--secure"), # default off
|
||||
("--secure", "--secure", "--no-secure"),
|
||||
("--no-secure", "--no-secure", "--secure"),
|
||||
("--not-secure", "--no-secure", "--secure"), # deprecated alias -> canonical
|
||||
],
|
||||
)
|
||||
def test_run_reexec_forwards_secure_polarity(monkeypatch, user_flag, expected, unexpected):
|
||||
|
|
@ -160,6 +172,31 @@ def test_studio_default_reexec_forwards_secure(monkeypatch):
|
|||
assert argv[argv.index("--host") + 1] == "127.0.0.1", argv
|
||||
|
||||
|
||||
def test_studio_default_not_secure_alias_forwards_no_secure(monkeypatch):
|
||||
# --not-secure on `unsloth studio` forwards the canonical --no-secure.
|
||||
captured = _invoke_studio_default(monkeypatch, ["--not-secure"])
|
||||
assert len(captured) == 1, captured
|
||||
argv = captured[0]
|
||||
assert "--no-secure" in argv and "--secure" not in argv, argv
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"argv_order,expected,unexpected",
|
||||
[
|
||||
# --not-secure tracks --no-secure: the last secure flag on argv wins,
|
||||
# matching the backend BooleanOptionalAction.
|
||||
(["--secure", "--not-secure"], "--no-secure", "--secure"),
|
||||
(["--not-secure", "--secure"], "--secure", "--no-secure"),
|
||||
],
|
||||
)
|
||||
def test_run_not_secure_alias_respects_last_wins(monkeypatch, argv_order, expected, unexpected):
|
||||
monkeypatch.setattr(sys, "argv", ["unsloth", "studio", "run", *argv_order])
|
||||
captured = _invoke_run(monkeypatch, _BASE + argv_order)
|
||||
assert len(captured) == 1, captured
|
||||
argv = captured[0]
|
||||
assert expected in argv and unexpected not in argv, argv
|
||||
|
||||
|
||||
# ── in-venv path forwards secure + forced host into run_server ────────
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue