Skip to content

Commit 7b049b9

Browse files
khushi0433i-yliu
authored andcommitted
fix(cli): enable interspersed option parsing for cloud_run command
The `cloud_run` CLI command was rejecting documented debug options (e.g., `--verbosity DEBUG`, `--log_level DEBUG`) when placed after positional arguments like `AGENT`, returning `Error: Unexpected arguments: --verbosity DEBUG`. Root Cause: The command definition previously disabled interspersed arguments (`allow_interspersed_args=False`), forcing manual parsing of the `--` separator to pass extra arguments to `gcloud`. This manual parsing was unnecessary and over-engineered, as Click natively consumes the `--` separator and delegates extra argument collection automatically when interspersed parsing is enabled. Solution: 1. Removed `allow_interspersed_args=False` from `context_settings` in `src/google/adk/cli/cli_tools_click.py` to allow Click's default interspersed option parsing. 2. Removed the obsolete manual `--` separator parsing logic, simplifying `gcloud_args` collection directly to `ctx.args`. Merge #4563 Original PR by @khushi0433 (khushbu_baloch <fatimahbaloch917@gmail.com>) Co-authored-by: Yi Liu <yiliuly@google.com> COPYBARA_INTEGRATE_REVIEW=#4563 from khushi0433:fix-cloud-run-verbosity 72b3485 PiperOrigin-RevId: 937424567
1 parent 62b9700 commit 7b049b9

2 files changed

Lines changed: 54 additions & 58 deletions

File tree

src/google/adk/cli/cli_tools_click.py

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -2000,7 +2000,6 @@ def cli_api_server(
20002000
"cloud_run",
20012001
context_settings={
20022002
"allow_extra_args": True,
2003-
"allow_interspersed_args": False,
20042003
},
20052004
)
20062005
@click.option(
@@ -2177,34 +2176,7 @@ def cli_deploy_cloud_run(
21772176

21782177
_warn_if_with_ui(with_ui)
21792178

2180-
# Parse arguments to separate gcloud args (after --) from regular args
2181-
gcloud_args = []
2182-
if "--" in ctx.args:
2183-
separator_index = ctx.args.index("--")
2184-
gcloud_args = ctx.args[separator_index + 1 :]
2185-
regular_args = ctx.args[:separator_index]
2186-
2187-
# If there are regular args before --, that's an error
2188-
if regular_args:
2189-
click.secho(
2190-
"Error: Unexpected arguments after agent path and before '--':"
2191-
f" {' '.join(regular_args)}. \nOnly arguments after '--' are passed"
2192-
" to gcloud.",
2193-
fg="red",
2194-
err=True,
2195-
)
2196-
ctx.exit(2)
2197-
else:
2198-
# No -- separator, treat all args as an error to enforce the new behavior
2199-
if ctx.args:
2200-
click.secho(
2201-
f"Error: Unexpected arguments: {' '.join(ctx.args)}. \nUse '--' to"
2202-
" separate gcloud arguments, e.g.: adk deploy cloud_run [options]"
2203-
" agent_path -- --min-instances=2",
2204-
fg="red",
2205-
err=True,
2206-
)
2207-
ctx.exit(2)
2179+
gcloud_args = ctx.args
22082180

22092181
try:
22102182
from . import cli_deploy

tests/unittests/cli/utils/test_cli_tools_click.py

Lines changed: 53 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -886,14 +886,14 @@ def test_cli_deploy_cloud_run_passthrough_args(
886886
assert "--cpu=1" in extra_args
887887

888888

889-
def test_cli_deploy_cloud_run_rejects_args_without_separator(
889+
def test_cli_deploy_cloud_run_allows_empty_gcloud_args(
890890
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
891891
) -> None:
892-
"""Args without '--' separator should be rejected with helpful error message."""
892+
"""No gcloud args after '--' should be allowed."""
893893
rec = _Recorder()
894894
monkeypatch.setattr("google.adk.cli.cli_deploy.to_cloud_run", rec)
895895

896-
agent_dir = tmp_path / "agent_no_sep"
896+
agent_dir = tmp_path / "agent_empty_gcloud"
897897
agent_dir.mkdir()
898898
runner = CliRunner()
899899
result = runner.invoke(
@@ -906,58 +906,87 @@ def test_cli_deploy_cloud_run_rejects_args_without_separator(
906906
"--region",
907907
"us-central1",
908908
str(agent_dir),
909-
"--labels=test-label=test", # This should be rejected
909+
"--",
910+
# No gcloud args after --
910911
],
911912
)
912913

913-
assert result.exit_code == 2
914-
assert "Unexpected arguments:" in result.output
915-
assert "Use '--' to separate gcloud arguments" in result.output
916-
assert not rec.calls, "cli_deploy.to_cloud_run should not be called"
914+
assert result.exit_code == 0
915+
assert rec.calls, "cli_deploy.to_cloud_run must be invoked"
916+
917+
# Check that extra_gcloud_args is empty
918+
called_kwargs = rec.calls[0][1]
919+
extra_args = called_kwargs.get("extra_gcloud_args")
920+
assert extra_args == ()
917921

918922

919-
def test_cli_deploy_cloud_run_rejects_args_before_separator(
923+
def test_cli_deploy_cloud_run_interspersed_options(
920924
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
921925
) -> None:
922-
"""Args before '--' separator should be rejected."""
926+
"""Options placed after the positional argument should be parsed correctly."""
923927
rec = _Recorder()
924928
monkeypatch.setattr("google.adk.cli.cli_deploy.to_cloud_run", rec)
925929

926-
agent_dir = tmp_path / "agent_before_sep"
930+
agent_dir = tmp_path / "agent_interspersed"
927931
agent_dir.mkdir()
928932
runner = CliRunner()
929933
result = runner.invoke(
930934
cli_tools_click.main,
931935
[
932936
"deploy",
933937
"cloud_run",
938+
str(agent_dir),
934939
"--project",
935940
"test-project",
936941
"--region",
937942
"us-central1",
943+
],
944+
)
945+
946+
assert result.exit_code == 0
947+
assert rec.calls, "cli_deploy.to_cloud_run must be invoked"
948+
949+
called_kwargs = rec.calls[0][1]
950+
assert called_kwargs.get("project") == "test-project"
951+
assert called_kwargs.get("region") == "us-central1"
952+
953+
954+
def test_cli_deploy_cloud_run_rejects_unknown_option_before_separator(
955+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
956+
) -> None:
957+
"""Unknown option placed before '--' separator should be rejected by Click."""
958+
rec = _Recorder()
959+
monkeypatch.setattr("google.adk.cli.cli_deploy.to_cloud_run", rec)
960+
961+
agent_dir = tmp_path / "agent_bad_order"
962+
agent_dir.mkdir()
963+
runner = CliRunner()
964+
result = runner.invoke(
965+
cli_tools_click.main,
966+
[
967+
"deploy",
968+
"cloud_run",
969+
"--project",
970+
"test-project",
938971
str(agent_dir),
939-
"unexpected_arg", # This should be rejected
940-
"--",
941972
"--labels=test-label=test",
973+
"--",
942974
],
943975
)
944976

945977
assert result.exit_code == 2
946-
assert (
947-
"Unexpected arguments after agent path and before '--':" in result.output
948-
)
949-
assert "unexpected_arg" in result.output
978+
assert "No such option" in result.output
950979
assert not rec.calls, "cli_deploy.to_cloud_run should not be called"
951980

952981

953-
def test_cli_deploy_cloud_run_allows_empty_gcloud_args(
982+
def test_cli_deploy_cloud_run_forwards_extra_positional_arg(
954983
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
955984
) -> None:
956-
"""No gcloud args after '--' should be allowed."""
985+
"""Extra positional argument before '--' is forwarded to the deployment runner."""
957986
rec = _Recorder()
958987
monkeypatch.setattr("google.adk.cli.cli_deploy.to_cloud_run", rec)
959988

960-
agent_dir = tmp_path / "agent_empty_gcloud"
989+
agent_dir = tmp_path / "agent_extra_pos"
961990
agent_dir.mkdir()
962991
runner = CliRunner()
963992
result = runner.invoke(
@@ -967,21 +996,16 @@ def test_cli_deploy_cloud_run_allows_empty_gcloud_args(
967996
"cloud_run",
968997
"--project",
969998
"test-project",
970-
"--region",
971-
"us-central1",
972999
str(agent_dir),
1000+
"unexpected_arg",
9731001
"--",
974-
# No gcloud args after --
1002+
"--labels=test-label=test",
9751003
],
9761004
)
9771005

9781006
assert result.exit_code == 0
979-
assert rec.calls, "cli_deploy.to_cloud_run must be invoked"
980-
981-
# Check that extra_gcloud_args is empty
982-
called_kwargs = rec.calls[0][1]
983-
extra_args = called_kwargs.get("extra_gcloud_args")
984-
assert extra_args == ()
1007+
extra_args = rec.calls[0][1].get("extra_gcloud_args")
1008+
assert extra_args == ("unexpected_arg", "--labels=test-label=test")
9851009

9861010

9871011
# cli deploy agent_engine

0 commit comments

Comments
 (0)