diff --git a/.gitignore b/.gitignore index 335b3214..2e9ad775 100644 --- a/.gitignore +++ b/.gitignore @@ -62,5 +62,6 @@ docs/_build/ # OS specific .DS_Store Thumbs.db +default.profraw tests/test_workspace diff --git a/packages/cli/src/pywrangler/cli.py b/packages/cli/src/pywrangler/cli.py index 890e8b28..046f70d7 100644 --- a/packages/cli/src/pywrangler/cli.py +++ b/packages/cli/src/pywrangler/cli.py @@ -8,6 +8,7 @@ import click from .sync import sync +from .unsupported_flags import find_unsupported_flag from .utils import ( WRANGLER_COMMAND, WRANGLER_CREATE_COMMAND, @@ -68,6 +69,11 @@ def get_command(self, ctx: click.Context, cmd_name: str) -> click.Command: except ValueError: remaining_args = [] + unsupported = find_unsupported_flag(cmd_name, remaining_args) + if unsupported is not None: + logger.error(unsupported.error_message()) + sys.exit(1) + if cmd_name in {"dev", "publish", "deploy", "versions", "preview"}: sync(force=False) diff --git a/packages/cli/src/pywrangler/unsupported_flags.py b/packages/cli/src/pywrangler/unsupported_flags.py new file mode 100644 index 00000000..028c96fc --- /dev/null +++ b/packages/cli/src/pywrangler/unsupported_flags.py @@ -0,0 +1,77 @@ +from collections.abc import Sequence +from dataclasses import dataclass + + +@dataclass(frozen=True) +class UnsupportedFlag: + """A wrangler flag that pywrangler rejects before proxying.""" + + command: str + names: tuple[str, ...] + hint: str + + def matches(self, cmd_name: str, args: Sequence[str]) -> bool: + if cmd_name != self.command: + return False + flag_args = _flag_args(args) + return any( + self._is_enabled(arg, flag_args[i + 1] if i + 1 < len(flag_args) else None) + for i, arg in enumerate(flag_args) + ) + + def _is_enabled(self, arg: str, next_arg: str | None) -> bool: + # Enabled by `--flag/-f`, unless followed by a separate `false` value + if arg in self.names: + return next_arg != "false" + # With `--flag=value`, enables the flag only for a literal `true` + # instestingly, wrangler ignores all other values other than `true/false`, + # so things like --flag=1 is the same as --flag=False + name, sep, value = arg.partition("=") + return bool(sep) and name in self.names and value == "true" + + def error_message(self) -> str: + return ( + f"`pywrangler {self.command} {self.names[0]}` is not supported " + f"for Python Workers.\n{self.hint}" + ) + + +UNSUPPORTED_FLAGS: tuple[UnsupportedFlag, ...] = ( + UnsupportedFlag( + command="dev", + names=("--remote", "-r"), + hint=( + "Please use remote bindings instead.\n" + "See https://developers.cloudflare.com/workers/development-testing/#remote-bindings" + ), + ), +) + + +def find_unsupported_flag( + cmd_name: str, + args: Sequence[str], + registry: Sequence[UnsupportedFlag] = UNSUPPORTED_FLAGS, +) -> UnsupportedFlag | None: + """ + Find an unsupported flag in the given command and arguments. + + Parameters + ---------- + cmd_name: + The command name to check. + args: + The arguments to check. + registry: + The registry of unsupported flags to check against. + (For testing purposes. Normally not needed.) + + Returns + ------- + The unsupported flag if found, otherwise None. + """ + return next((flag for flag in registry if flag.matches(cmd_name, args)), None) + + +def _flag_args(args: Sequence[str]) -> Sequence[str]: + return args[: args.index("--")] if "--" in args else args diff --git a/packages/cli/tests/test_cli.py b/packages/cli/tests/test_cli.py index 71c578fb..2bb62986 100644 --- a/packages/cli/tests/test_cli.py +++ b/packages/cli/tests/test_cli.py @@ -690,6 +690,60 @@ def test_proxy_auto_sync_commands( mock_proxy_to_wrangler.assert_called_once_with("dev", ["--local"]) +@pytest.mark.parametrize( + "remote_args", + [ + ["--remote"], + ["-r"], + ["--port", "8787", "--remote"], + ["--remote=true"], + ["--remote", "true"], + ], +) +@patch("pywrangler.utils.check_wrangler_version") +@patch("pywrangler.cli._proxy_to_wrangler") +@patch("pywrangler.cli.sync") +def test_dev_remote_is_rejected( + mock_sync_command, + mock_proxy_to_wrangler, + mock_check_wrangler_version, + remote_args, + caplog, +): + with patch("sys.argv", ["pywrangler", "dev", *remote_args]): + result = CliRunner().invoke(app, ["dev", *remote_args]) + + assert result.exit_code == 1 + assert "remote bindings" in caplog.text + mock_sync_command.assert_not_called() + mock_proxy_to_wrangler.assert_not_called() + + +@pytest.mark.parametrize( + "args", [["--remote", "false"], ["-r", "false"], ["--remote=false"]] +) +@patch("pywrangler.utils.check_wrangler_version") +@patch("pywrangler.cli._proxy_to_wrangler") +@patch("pywrangler.cli.sync") +def test_dev_remote_disabled_is_proxied( + mock_sync_command, mock_proxy_to_wrangler, mock_check_wrangler_version, args +): + with patch("sys.argv", ["pywrangler", "dev", *args]): + result = CliRunner().invoke(app, ["dev", *args]) + + assert result.exit_code == 0 + mock_proxy_to_wrangler.assert_called_once_with("dev", args) + + +@patch("pywrangler.cli._proxy_to_wrangler") +@patch("sys.argv", ["pywrangler", "d1", "execute", "db", "--remote"]) +def test_remote_flag_allowed_for_non_dev_commands(mock_proxy_to_wrangler): + result = CliRunner().invoke(app, ["d1", "execute", "db", "--remote"]) + + assert result.exit_code == 0 + mock_proxy_to_wrangler.assert_called_once_with("d1", ["execute", "db", "--remote"]) + + @patch("pywrangler.cli.subprocess.run") def test_proxy_to_wrangler_handles_subprocess_error(mock_subprocess_run): """Test that subprocess errors are handled gracefully.""" diff --git a/packages/cli/tests/test_unsupported_flags.py b/packages/cli/tests/test_unsupported_flags.py new file mode 100644 index 00000000..028bcfd6 --- /dev/null +++ b/packages/cli/tests/test_unsupported_flags.py @@ -0,0 +1,70 @@ +import pytest + +from pywrangler.unsupported_flags import ( + UNSUPPORTED_FLAGS, + UnsupportedFlag, + find_unsupported_flag, +) + +FLAG = UnsupportedFlag(command="dev", names=("--foo", "-f"), hint="Do X instead.") +REGISTRY = (FLAG,) + + +@pytest.mark.parametrize( + "args", + [ + ["--foo"], + ["-f"], + ["--port", "8787", "--foo"], + ["--foo=true"], + ["-f=true"], + ["--foo", "true"], + ["--foo", "--port", "8787"], + ], +) +def test_enabled_flag_is_found(args): + assert find_unsupported_flag("dev", args, REGISTRY) is FLAG + + +@pytest.mark.parametrize( + "args", + [ + [], + ["--foobar"], + ["--foo=false"], + ["--foo=0"], + ["--foo=No"], + ["--foo=1"], + ["--foo=yes"], + ["--foo=TRUE"], + ["--foo="], + ["-f=0"], + ["--foo", "false"], + ["-f", "false"], + ["--foo", "false", "--port", "8787"], + ["--", "--foo"], + ], +) +def test_disabled_or_absent_flag_is_ignored(args): + assert find_unsupported_flag("dev", args, REGISTRY) is None + + +def test_flag_is_scoped_to_its_command(): + assert find_unsupported_flag("deploy", ["--foo"], REGISTRY) is None + + +def test_error_message(): + assert FLAG.error_message() == ( + "`pywrangler dev --foo` is not supported for Python Workers.\nDo X instead." + ) + + +@pytest.mark.parametrize("args", [["--remote"], ["-r"], ["--remote=true"]]) +def test_default_registry_rejects_dev_remote(args): + flag = find_unsupported_flag("dev", args, UNSUPPORTED_FLAGS) + assert flag is not None + assert "remote bindings" in flag.error_message() + + +def test_default_registry_allows_remote_for_other_commands(): + assert find_unsupported_flag("d1", ["execute", "db", "--remote"]) is None