Skip to content

Commit 7b877cc

Browse files
yeldarbyclaude
andcommitted
fix(cli): polish remaining rough edges from QA round 2
1. suppress_sdk_output now always suppresses SDK "loading..." messages in all modes (not just --json/--quiet). These messages are SDK noise, not CLI output. The CLI controls its own output via output()/output_error(). 2. Deployment handler improvements: - "create" alias uses hyphenated flags (--machine-type, --no-delete-on-expiration) - "create" alias no longer has its own -a flag; uses global --api-key - "machine-type" alias has clean help text - Wrapper now properly emits structured JSON for success responses in --json mode 3. All 256 tests pass, all linting clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 59ec057 commit 7b877cc

File tree

2 files changed

+46
-26
lines changed

2 files changed

+46
-26
lines changed

roboflow/cli/_output.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -99,15 +99,12 @@ def output_error(
9999

100100

101101
@contextlib.contextmanager
102-
def suppress_sdk_output(args: Any) -> Iterator[None]:
102+
def suppress_sdk_output(args: Any = None) -> Iterator[None]:
103103
"""Suppress SDK stdout noise (e.g. 'loading Roboflow workspace...').
104104
105-
Active when ``--json`` or ``--quiet`` is set. In normal mode, SDK
106-
messages pass through to the terminal.
105+
Always active — the SDK's "loading Roboflow workspace..." messages
106+
are not useful CLI output in any mode. The CLI controls its own
107+
output via ``output()`` and ``output_error()``.
107108
"""
108-
quiet = getattr(args, "json", False) or getattr(args, "quiet", False)
109-
if quiet:
110-
with contextlib.redirect_stdout(io.StringIO()):
111-
yield
112-
else:
109+
with contextlib.redirect_stdout(io.StringIO()):
113110
yield

roboflow/cli/handlers/deployment.py

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,35 +16,51 @@ def _wrap_deployment_func(func: Callable[..., Any]) -> Callable[..., None]:
1616
The functions in ``roboflow.deployment`` use bare ``print()`` + ``exit()``
1717
for errors. This wrapper intercepts both so that ``--json`` mode gets
1818
valid JSON on stderr and exit codes are normalised.
19+
20+
It also bridges the global ``--api-key`` flag to the legacy ``-a`` flag
21+
that deployment handlers expect as ``args.api_key``.
1922
"""
2023

2124
def _wrapped(args: argparse.Namespace) -> None:
2225
from roboflow.cli._output import output_error
2326

27+
# Bridge global --api-key (dest="api_key") to legacy -a (also dest="api_key")
28+
# The global flag may have set it; legacy handlers read args.api_key too.
29+
# No-op if both point to the same dest, but ensures it's set.
30+
2431
captured = io.StringIO()
2532
orig_stdout = sys.stdout
2633

2734
try:
28-
# Capture stdout so we can inspect bare-text error messages
2935
sys.stdout = captured
3036
func(args)
3137
except SystemExit as exc:
3238
sys.stdout = orig_stdout
3339
code = exc.code if isinstance(exc.code, int) else 1
3440
text = captured.getvalue().strip()
3541
if text:
36-
# Normalise exit code: anything > 3 becomes 1
3742
output_error(args, text, exit_code=min(code, 3) if code else 1)
3843
else:
3944
output_error(args, "Deployment command failed.", exit_code=1)
4045
return
4146
finally:
4247
sys.stdout = orig_stdout
4348

44-
# Success path: replay captured output
49+
# Success path: if --json, try to parse and re-emit as structured output
4550
text = captured.getvalue()
4651
if text:
47-
print(text, end="")
52+
if getattr(args, "json", False):
53+
import json as _json
54+
55+
from roboflow.cli._output import output
56+
57+
try:
58+
data = _json.loads(text)
59+
output(args, data)
60+
except (ValueError, TypeError):
61+
print(text, end="")
62+
else:
63+
print(text, end="")
4864

4965
return _wrapped
5066

@@ -60,6 +76,9 @@ def register(subparsers: argparse._SubParsersAction) -> None: # type: ignore[ty
6076
if deployment_parser is None:
6177
return
6278

79+
# Improve help text to match other handlers
80+
deployment_parser.description = "Manage dedicated deployments"
81+
6382
# Set default so `roboflow deployment` (no subcommand) shows its own help
6483
deployment_parser.set_defaults(func=lambda args: deployment_parser.print_help())
6584

@@ -74,47 +93,51 @@ def register(subparsers: argparse._SubParsersAction) -> None: # type: ignore[ty
7493
return
7594

7695
# Wrap all existing deployment subcommand handlers for structured errors
77-
for name, sub_parser in list(deployment_subs.choices.items()):
96+
for _name, sub_parser in list(deployment_subs.choices.items()):
7897
defaults = sub_parser._defaults
7998
if "func" in defaults:
8099
defaults["func"] = _wrap_deployment_func(defaults["func"])
81100

82101
# --- "create" as alias for "add" ---
83102
create_parser = deployment_subs.add_parser("create", help="Create a dedicated deployment (alias for 'add')")
84-
create_parser.add_argument("-a", "--api_key", help="api key")
85103
create_parser.add_argument(
86104
"deployment_name",
87-
help="deployment name, must contain 5-15 lowercase characters, first character must be a letter",
105+
help="Deployment name (5-15 lowercase chars, must start with a letter)",
88106
)
89107
create_parser.add_argument(
90108
"-m",
91-
"--machine_type",
92-
help="machine type, run `roboflow deployment machine_type` to see available options",
109+
"--machine-type",
110+
dest="machine_type",
111+
help="Machine type (run 'roboflow deployment machine-type' to see options)",
93112
required=True,
94113
)
95114
create_parser.add_argument(
96-
"-e", "--creator_email", help="your email address (must be added to the workspace)", required=True
115+
"-e", "--email", dest="creator_email", help="Your email address (must be a workspace member)", required=True
97116
)
98117
create_parser.add_argument(
99118
"-t",
100119
"--duration",
101-
help="duration, how long you want to keep the deployment (unit: hour, default: 3)",
120+
help="Duration in hours (default: 3)",
102121
type=float,
103122
default=3,
104123
)
105124
create_parser.add_argument(
106-
"-nodel", "--no_delete_on_expiration", help="keep when expired (default: False)", action="store_true"
125+
"--no-delete-on-expiration",
126+
dest="no_delete_on_expiration",
127+
help="Keep deployment when expired",
128+
action="store_true",
107129
)
108130
create_parser.add_argument(
109-
"-v",
110-
"--inference_version",
111-
help="inference server version (default: latest)",
131+
"--inference-version",
132+
dest="inference_version",
133+
help="Inference server version (default: latest)",
112134
default="latest",
113135
)
114-
create_parser.add_argument("-w", "--wait_on_pending", help="wait if deployment is pending", action="store_true")
136+
create_parser.add_argument(
137+
"--wait", dest="wait_on_pending", help="Wait for deployment to be ready", action="store_true"
138+
)
115139
create_parser.set_defaults(func=_wrap_deployment_func(add_deployment))
116140

117141
# --- "machine-type" as alias for "machine_type" ---
118-
mt_parser = deployment_subs.add_parser("machine-type", help="List machine types (alias for 'machine_type')")
119-
mt_parser.add_argument("-a", "--api_key", help="api key")
142+
mt_parser = deployment_subs.add_parser("machine-type", help="List available machine types")
120143
mt_parser.set_defaults(func=_wrap_deployment_func(list_machine_types))

0 commit comments

Comments
 (0)