Skip to content

Commit 0187095

Browse files
committed
Make media_types keyword-only in the adapter and tighten the export guard
Two corrections to the previous commits. `rfapi.workspace_search` took `media_types` as a positional-or-keyword parameter, so the "keyword-only everywhere" claim was only true of the higher-level SDK methods. Put it behind a bare `*`. The six pre-existing parameters keep their positions, and a test now pins both the kind and the positional order. The CLI export guard tested `--media-types` for truthiness, so an explicit but empty `--media-types ""` was falsy and fell through to the export route instead of being rejected — verified by dispatch: the old guard got as far as resolving the export dataset. Compare against `None` so any explicit value is caught, and reject the blank value on the search path too. `parse_media_types_option` also documented an empty string as meaning "use the API default" while the code raised on it. The code is right — a blank value is an explicit empty selection — so the docstring now says that.
1 parent 902c000 commit 0187095

5 files changed

Lines changed: 71 additions & 5 deletions

File tree

‎roboflow/adapters/rfapi.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,7 @@ def workspace_search(
546546
page_size: int = 50,
547547
fields: Optional[List[str]] = None,
548548
continuation_token: Optional[str] = None,
549+
*,
549550
media_types: Optional[List[str]] = None,
550551
) -> dict:
551552
"""Search across all media in a workspace using RoboQL syntax.

‎roboflow/cli/handlers/search.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,9 @@ def _search(args): # noqa: ANN001
8080
return
8181

8282
if args.export:
83-
if getattr(args, "media_types", None):
83+
# `is not None` on purpose: an explicit but invalid value (e.g. --media-types "")
84+
# must not silently fall through to the export route.
85+
if getattr(args, "media_types", None) is not None:
8486
output_error(
8587
args,
8688
"--media-types is not supported with --export",

‎roboflow/util/search_utils.py‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,16 @@ def parse_media_types_option(raw: Optional[str]) -> Optional[List[str]]:
5151
"""Parse a comma-separated CLI ``--media-types`` value into a normalized list.
5252
5353
Args:
54-
raw: Raw option value, e.g. ``"video"`` or ``"image,video"``. ``None`` or an
55-
empty string means "use the API default".
54+
raw: Raw option value, e.g. ``"video"`` or ``"image,video"``. Only ``None``
55+
(the option was not given) means "use the API default".
5656
5757
Returns:
58-
A normalized media type list, or ``None`` when nothing was requested.
58+
A normalized media type list, or ``None`` when the option was not given.
5959
6060
Raises:
61-
ValueError: If the value contains anything other than valid media types.
61+
ValueError: If the value is given but blank, or names anything other than a
62+
valid media type. A blank value is an explicit empty selection, not a
63+
request for the default.
6264
"""
6365
if raw is None:
6466
return None

‎tests/cli/test_search_media_types.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,44 @@ def test_export_rejects_media_types(self):
179179
error = json.loads(result.output)["error"]
180180
self.assertIn("--media-types is not supported with --export", error["message"])
181181

182+
@responses.activate
183+
def test_export_rejects_an_explicitly_blank_media_types(self):
184+
"""A blank value is an explicit selection; it must not fall through to the export route."""
185+
_stub_workspace_init()
186+
187+
result = runner.invoke(
188+
app,
189+
[
190+
"--json",
191+
"--api-key",
192+
API_KEY,
193+
"--workspace",
194+
WORKSPACE,
195+
"search",
196+
"*",
197+
"--export",
198+
"--media-types",
199+
"",
200+
],
201+
)
202+
203+
self.assertEqual(result.exit_code, 1)
204+
error = json.loads(result.output)["error"]
205+
self.assertIn("--media-types is not supported with --export", error["message"])
206+
207+
@responses.activate
208+
def test_blank_media_types_without_export_is_rejected(self):
209+
_stub_workspace_init()
210+
211+
result = runner.invoke(
212+
app,
213+
["--json", "--api-key", API_KEY, "--workspace", WORKSPACE, "search", "*", "--media-types", ""],
214+
)
215+
216+
self.assertEqual(result.exit_code, 1)
217+
self.assertEqual(_search_bodies(), [])
218+
self.assertIn("at least one of", json.loads(result.output)["error"]["message"])
219+
182220

183221
class TestImageSearchMediaTypes(unittest.TestCase):
184222
"""`roboflow image search` covers both the workspace and project-scoped paths."""

‎tests/test_search_media_types.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,29 @@ def test_adapter_sends_media_types(self):
242242

243243
self.assertEqual(self._sent()["mediaTypes"], ["video"])
244244

245+
def test_adapter_media_types_is_keyword_only(self):
246+
"""Positional callers must keep working, so media_types sits behind a bare `*`."""
247+
import inspect
248+
249+
sig = inspect.signature(rfapi.workspace_search)
250+
self.assertEqual(sig.parameters["media_types"].kind, inspect.Parameter.KEYWORD_ONLY)
251+
# The pre-existing parameters stay positional-or-keyword, in their original order.
252+
positional = [n for n, p in sig.parameters.items() if p.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD]
253+
self.assertEqual(
254+
positional,
255+
["api_key", "workspace_url", "query", "page_size", "fields", "continuation_token"],
256+
)
257+
258+
def test_sdk_search_media_types_is_keyword_only(self):
259+
import inspect
260+
261+
from roboflow.core.project import Project
262+
from roboflow.core.workspace import Workspace
263+
264+
for func in (Project.search, Project.search_all, Workspace.search, Workspace.search_all):
265+
kind = inspect.signature(func).parameters["media_types"].kind
266+
self.assertEqual(kind, inspect.Parameter.KEYWORD_ONLY, func.__qualname__)
267+
245268
@responses.activate
246269
def test_adapter_without_selection_omits_media_types(self):
247270
responses.add(responses.POST, self.SEARCH_URL, json={"results": [], "total": 0}, status=200)

0 commit comments

Comments
 (0)