From 0b3d2c6223e3330b5be7d4f8951fc8beadb9b236 Mon Sep 17 00:00:00 2001 From: Christoph Deil Date: Wed, 7 Oct 2026 16:11:47 +0200 Subject: [PATCH] fix(cli): keep stdout valid JSON when SDK calls print in --json mode search --export, image upload , model upload and version download call SDK methods that print progress and status lines to stdout, so stdout was a mix of plain text and the JSON result. In --json mode these calls now run with stdout redirected to stderr, where the progress stays visible. model upload also silences the "loading Roboflow workspace..." message during workspace init, like the other handlers. Co-Authored-By: Claude Opus 5.5 --- roboflow/cli/_output.py | 16 ++++ roboflow/cli/handlers/image.py | 29 +++---- roboflow/cli/handlers/model.py | 30 ++++--- roboflow/cli/handlers/search.py | 21 ++--- roboflow/cli/handlers/version.py | 5 +- tests/cli/test_json_stdout.py | 134 +++++++++++++++++++++++++++++++ 6 files changed, 196 insertions(+), 39 deletions(-) create mode 100644 tests/cli/test_json_stdout.py diff --git a/roboflow/cli/_output.py b/roboflow/cli/_output.py index 70ba2c6c..c889cd5d 100644 --- a/roboflow/cli/_output.py +++ b/roboflow/cli/_output.py @@ -281,3 +281,19 @@ def suppress_sdk_output(args: Any = None) -> Iterator[None]: """ with contextlib.redirect_stdout(io.StringIO()): yield + + +@contextlib.contextmanager +def sdk_output_to_stderr(args: Any) -> Iterator[None]: + """Move SDK stdout output to stderr in ``--json`` mode. + + Some SDK calls print progress or per-item status that is worth seeing, + for example the per-image results of a directory upload, so it is not + suppressed. With ``--json`` it goes to stderr, so that stdout carries + only the JSON document written by ``output()``. + """ + if getattr(args, "json", False): + with contextlib.redirect_stdout(sys.stderr): + yield + else: + yield diff --git a/roboflow/cli/handlers/image.py b/roboflow/cli/handlers/image.py index 8d3a2786..90cd0980 100644 --- a/roboflow/cli/handlers/image.py +++ b/roboflow/cli/handlers/image.py @@ -359,7 +359,7 @@ def _handle_upload_directory(args, api_key: str, path: str) -> None: # noqa: AN import os import roboflow - from roboflow.cli._output import output, output_error, suppress_sdk_output + from roboflow.cli._output import output, output_error, sdk_output_to_stderr, suppress_sdk_output # Always suppress SDK "loading..." noise during workspace init with suppress_sdk_output(): @@ -376,19 +376,20 @@ def _handle_upload_directory(args, api_key: str, path: str) -> None: # noqa: AN wait = not getattr(args, "no_wait", False) try: - result = workspace.upload_dataset( - dataset_path=path, - project_name=args.project, - num_workers=args.concurrency, - batch_name=getattr(args, "batch", None), - num_retries=retries, - is_prediction=getattr(args, "is_prediction", False), - use_zip_upload=getattr(args, "zip_upload", False), - annotation_overwrite=getattr(args, "annotation_overwrite", None), - split=getattr(args, "split", None), - tags=tags, - wait=wait, - ) + with sdk_output_to_stderr(args): + result = workspace.upload_dataset( + dataset_path=path, + project_name=args.project, + num_workers=args.concurrency, + batch_name=getattr(args, "batch", None), + num_retries=retries, + is_prediction=getattr(args, "is_prediction", False), + use_zip_upload=getattr(args, "zip_upload", False), + annotation_overwrite=getattr(args, "annotation_overwrite", None), + split=getattr(args, "split", None), + tags=tags, + wait=wait, + ) except Exception as exc: output_error(args, str(exc)) return diff --git a/roboflow/cli/handlers/model.py b/roboflow/cli/handlers/model.py index 2f304fc7..536d93c5 100644 --- a/roboflow/cli/handlers/model.py +++ b/roboflow/cli/handlers/model.py @@ -311,11 +311,13 @@ def _get_model(args): # noqa: ANN001 def _upload_model(args): # noqa: ANN001 import roboflow - from roboflow.cli._output import output, output_error + from roboflow.cli._output import output, output_error, sdk_output_to_stderr, suppress_sdk_output api_key = args.api_key or None - rf = roboflow.Roboflow(api_key=api_key) - workspace = rf.workspace(args.workspace) + # Always suppress SDK "loading..." noise during workspace init + with suppress_sdk_output(): + rf = roboflow.Roboflow(api_key=api_key) + workspace = rf.workspace(args.workspace) if args.version_number is not None: # Deploy to a specific version @@ -325,9 +327,10 @@ def _upload_model(args): # noqa: ANN001 return try: - project = workspace.project(project_id) - version = project.version(args.version_number) - version.deploy(str(args.model_type), str(args.model_path), str(args.filename)) + with sdk_output_to_stderr(args): + project = workspace.project(project_id) + version = project.version(args.version_number) + version.deploy(str(args.model_type), str(args.model_path), str(args.filename)) except Exception as exc: output_error(args, str(exc)) return @@ -338,13 +341,14 @@ def _upload_model(args): # noqa: ANN001 return try: - workspace.deploy_model( - model_type=str(args.model_type), - model_path=str(args.model_path), - project_ids=args.project, - model_name=str(args.model_name) if args.model_name else "", - filename=str(args.filename), - ) + with sdk_output_to_stderr(args): + workspace.deploy_model( + model_type=str(args.model_type), + model_path=str(args.model_path), + project_ids=args.project, + model_name=str(args.model_name) if args.model_name else "", + filename=str(args.filename), + ) except Exception as exc: output_error(args, str(exc)) return diff --git a/roboflow/cli/handlers/search.py b/roboflow/cli/handlers/search.py index 3aec5eaf..48769c02 100644 --- a/roboflow/cli/handlers/search.py +++ b/roboflow/cli/handlers/search.py @@ -149,18 +149,19 @@ def _describe_hit(hit: dict) -> str: def _do_export(args: Any, workspace: Any) -> None: - from roboflow.cli._output import output, output_error + from roboflow.cli._output import output, output_error, sdk_output_to_stderr try: - result_path = workspace.search_export( - query=args.query, - format=args.format, - location=args.location, - dataset=args.dataset, - annotation_group=getattr(args, "annotation_group", None), - name=args.name, - extract_zip=not args.no_extract, - ) + with sdk_output_to_stderr(args): + result_path = workspace.search_export( + query=args.query, + format=args.format, + location=args.location, + dataset=args.dataset, + annotation_group=getattr(args, "annotation_group", None), + name=args.name, + extract_zip=not args.no_extract, + ) except Exception as exc: output_error(args, str(exc)) return diff --git a/roboflow/cli/handlers/version.py b/roboflow/cli/handlers/version.py index d1bd51d6..4153791b 100644 --- a/roboflow/cli/handlers/version.py +++ b/roboflow/cli/handlers/version.py @@ -230,7 +230,7 @@ def _parse_url(url: str) -> tuple: def _download(args): # noqa: ANN001 import roboflow - from roboflow.cli._output import output, output_error, suppress_sdk_output + from roboflow.cli._output import output, output_error, sdk_output_to_stderr, suppress_sdk_output w, p, v = _parse_url(args.url_or_id) @@ -257,7 +257,8 @@ def _download(args): # noqa: ANN001 else: version_obj = project.version(int(v)) - version_obj.download(args.format, location=args.location, overwrite=True) + with sdk_output_to_stderr(args): + version_obj.download(args.format, location=args.location, overwrite=True) except SystemExit: raise except Exception as exc: diff --git a/tests/cli/test_json_stdout.py b/tests/cli/test_json_stdout.py new file mode 100644 index 00000000..5310d8de --- /dev/null +++ b/tests/cli/test_json_stdout.py @@ -0,0 +1,134 @@ +"""stdout stays valid JSON in --json mode when SDK calls print progress.""" + +import contextlib +import io +import json +import os +import tempfile +import types +import unittest +from unittest.mock import MagicMock, patch + + +def _sdk_print(*_args: object, **_kwargs: object) -> None: + print("progress line printed by the SDK") + + +def _run(handler, *args: object) -> tuple: # noqa: ANN001 + stdout, stderr = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + handler(*args) + return stdout.getvalue(), stderr.getvalue() + + +class TestJsonStdout(unittest.TestCase): + def assert_json_stdout(self, stdout: str, stderr: str) -> dict: + self.assertNotIn("progress line printed by the SDK", stdout) + self.assertIn("progress line printed by the SDK", stderr) + return json.loads(stdout) + + def test_search_export(self) -> None: + from roboflow.cli.handlers.search import _do_export + + def search_export(**_kwargs: object) -> str: + _sdk_print() + return "export-dir" + + workspace = MagicMock() + workspace.search_export.side_effect = search_export + args = types.SimpleNamespace( + json=True, + query="tag:a", + format="coco", + location="export-dir", + dataset=None, + annotation_group=None, + name=None, + no_extract=False, + ) + + stdout, stderr = _run(_do_export, args, workspace) + + self.assertEqual(self.assert_json_stdout(stdout, stderr)["status"], "completed") + + @patch("roboflow.Roboflow") + def test_image_upload_directory(self, mock_rf_cls: MagicMock) -> None: + from roboflow.cli.handlers.image import _handle_upload_directory + + mock_rf_cls.return_value.workspace.return_value.upload_dataset.side_effect = _sdk_print + with tempfile.TemporaryDirectory() as tmpdir: + with open(os.path.join(tmpdir, "a.jpg"), "w") as f: + f.write("x") + args = types.SimpleNamespace( + json=True, + workspace="ws", + project="proj", + concurrency=1, + retries=0, + tag=None, + batch=None, + split=None, + is_prediction=False, + zip_upload=False, + annotation_overwrite=None, + no_wait=False, + ) + + stdout, stderr = _run(_handle_upload_directory, args, "key", tmpdir) + + self.assertEqual(self.assert_json_stdout(stdout, stderr)["count"], 1) + + @patch("roboflow.Roboflow") + def test_model_upload(self, mock_rf_cls: MagicMock) -> None: + from roboflow.cli.handlers.model import _upload_model + + def workspace(*_args: object) -> MagicMock: + print("loading Roboflow workspace...") + mock_workspace = MagicMock() + mock_workspace.project.return_value.version.return_value.deploy.side_effect = _sdk_print + return mock_workspace + + mock_rf_cls.return_value.workspace.side_effect = workspace + args = types.SimpleNamespace( + json=True, + api_key="key", + workspace="ws", + project=["proj"], + version_number=1, + model_type="yolov8", + model_path="/path/to/model", + filename="weights/best.pt", + model_name=None, + ) + + stdout, stderr = _run(_upload_model, args) + + self.assertEqual(self.assert_json_stdout(stdout, stderr)["status"], "uploaded") + self.assertNotIn("loading Roboflow workspace", stdout + stderr) + + @patch("roboflow.Roboflow") + def test_version_download(self, mock_rf_cls: MagicMock) -> None: + from roboflow.cli.handlers.version import _download + + project = mock_rf_cls.return_value.workspace.return_value.project.return_value + project.version.return_value.download.side_effect = _sdk_print + args = types.SimpleNamespace(json=True, url_or_id="ws/proj/1", format="coco", location="dataset-dir") + + stdout, stderr = _run(_download, args) + + self.assertEqual(self.assert_json_stdout(stdout, stderr)["version"], 1) + + def test_text_mode_keeps_sdk_output_on_stdout(self) -> None: + from roboflow.cli._output import sdk_output_to_stderr + + stdout, stderr = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + with sdk_output_to_stderr(types.SimpleNamespace(json=False)): + _sdk_print() + + self.assertIn("progress line printed by the SDK", stdout.getvalue()) + self.assertEqual(stderr.getvalue(), "") + + +if __name__ == "__main__": + unittest.main()