Skip to content

Commit aeb4dc1

Browse files
yeldarbyclaude
andcommitted
fix(cli): address code review findings #1, #3, #4
- #1 MUST-FIX: Fix Workspace.create_workflow to map definition->config and drop unsupported description param - #3 SHOULD-FIX: Delegate Project.get_annotation_jobs/get_annotation_job to rfapi instead of inline HTTP calls - #4 SHOULD-FIX: Extract resolve_ws_and_key to _resolver.py, deduplicate from folder/workspace/workflow handlers - Update test to expect exit code 2 (auth error) for missing workspace Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 9ef7c9e commit aeb4dc1

7 files changed

Lines changed: 40 additions & 72 deletions

File tree

roboflow/cli/_resolver.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,3 +113,25 @@ def resolve_resource(
113113
f"Cannot resolve '{shorthand}': expected 1-3 path segments "
114114
"(project, workspace/project, or workspace/project/version)."
115115
)
116+
117+
118+
def resolve_ws_and_key(args) -> Optional[Tuple[str, str]]:
119+
"""Resolve workspace and API key from CLI args.
120+
121+
Returns (workspace_url, api_key) or ``None`` after calling
122+
``output_error`` on failure.
123+
"""
124+
from roboflow.cli._output import output_error
125+
from roboflow.config import load_roboflow_api_key
126+
127+
ws = getattr(args, "workspace", None) or resolve_default_workspace(api_key=getattr(args, "api_key", None))
128+
if not ws:
129+
output_error(args, "No workspace specified.", hint="Use --workspace or run 'roboflow auth login'.", exit_code=2)
130+
return None
131+
132+
api_key = getattr(args, "api_key", None) or load_roboflow_api_key(ws)
133+
if not api_key:
134+
output_error(args, "No API key found.", hint="Set ROBOFLOW_API_KEY or run 'roboflow auth login'.", exit_code=2)
135+
return None
136+
137+
return ws, api_key

roboflow/cli/handlers/folder.py

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,21 +46,9 @@ def register(subparsers: argparse._SubParsersAction) -> None: # type: ignore[ty
4646

4747
def _resolve_ws_and_key(args: argparse.Namespace):
4848
"""Resolve workspace and API key, returning (ws, api_key) or None on error."""
49-
from roboflow.cli._output import output_error
50-
from roboflow.cli._resolver import resolve_default_workspace
51-
from roboflow.config import load_roboflow_api_key
49+
from roboflow.cli._resolver import resolve_ws_and_key
5250

53-
ws = args.workspace or resolve_default_workspace(api_key=args.api_key)
54-
if not ws:
55-
output_error(args, "No workspace specified.", hint="Use --workspace or run 'roboflow auth login'.", exit_code=2)
56-
return None
57-
58-
api_key = args.api_key or load_roboflow_api_key(ws)
59-
if not api_key:
60-
output_error(args, "No API key found.", hint="Set ROBOFLOW_API_KEY or run 'roboflow auth login'.", exit_code=2)
61-
return None
62-
63-
return ws, api_key
51+
return resolve_ws_and_key(args)
6452

6553

6654
def _list_folders(args: argparse.Namespace) -> None:

roboflow/cli/handlers/workflow.py

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -75,24 +75,9 @@ def register(subparsers: argparse._SubParsersAction) -> None: # type: ignore[ty
7575

7676
def _resolve_workspace_and_key(args: argparse.Namespace):
7777
"""Return (workspace_url, api_key) or call output_error and return None."""
78-
from roboflow.cli._output import output_error
79-
from roboflow.cli._resolver import resolve_default_workspace
80-
from roboflow.config import load_roboflow_api_key
81-
82-
workspace_url = args.workspace
83-
if not workspace_url:
84-
workspace_url = resolve_default_workspace(api_key=args.api_key)
85-
86-
if not workspace_url:
87-
output_error(args, "No workspace specified.", hint="Use --workspace or run 'roboflow auth login'.")
88-
return None
89-
90-
api_key = args.api_key or load_roboflow_api_key(workspace_url)
91-
if not api_key:
92-
output_error(args, "No API key found.", hint="Set ROBOFLOW_API_KEY or run 'roboflow auth login'.", exit_code=2)
93-
return None
78+
from roboflow.cli._resolver import resolve_ws_and_key
9479

95-
return workspace_url, api_key
80+
return resolve_ws_and_key(args)
9681

9782

9883
def _read_definition_file(args: argparse.Namespace):

roboflow/cli/handlers/workspace.py

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -133,21 +133,9 @@ def _get_workspace(args: argparse.Namespace) -> None:
133133

134134
def _resolve_ws_and_key(args: argparse.Namespace):
135135
"""Resolve workspace and API key for workspace subcommands."""
136-
from roboflow.cli._output import output_error
137-
from roboflow.cli._resolver import resolve_default_workspace
138-
from roboflow.config import load_roboflow_api_key
139-
140-
ws = args.workspace or resolve_default_workspace(api_key=args.api_key)
141-
if not ws:
142-
output_error(args, "No workspace specified.", hint="Use --workspace or run 'roboflow auth login'.", exit_code=2)
143-
return None
144-
145-
api_key = args.api_key or load_roboflow_api_key(ws)
146-
if not api_key:
147-
output_error(args, "No API key found.", hint="Set ROBOFLOW_API_KEY or run 'roboflow auth login'.", exit_code=2)
148-
return None
136+
from roboflow.cli._resolver import resolve_ws_and_key
149137

150-
return ws, api_key
138+
return resolve_ws_and_key(args)
151139

152140

153141
def _workspace_usage(args: argparse.Namespace) -> None:

roboflow/core/project.py

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -878,17 +878,9 @@ def get_annotation_jobs(self) -> Dict:
878878
Returns:
879879
Dict: A dictionary containing the list of annotation jobs.
880880
"""
881-
url = f"{API_URL}/{self.__workspace}/{self.__project_name}/jobs?api_key={self.__api_key}"
882-
response = requests.get(url)
883-
if response.status_code != 200:
884-
try:
885-
error_data = response.json()
886-
if "error" in error_data:
887-
raise RuntimeError(error_data["error"])
888-
raise RuntimeError(response.text)
889-
except ValueError:
890-
raise RuntimeError(f"Failed to get annotation jobs: {response.text}")
891-
return response.json()
881+
from roboflow.adapters import rfapi
882+
883+
return rfapi.list_annotation_jobs(self.__api_key, self.__workspace, self.__project_name)
892884

893885
def get_annotation_job(self, job_id: str) -> Dict:
894886
"""Get information for a specific annotation job.
@@ -899,17 +891,9 @@ def get_annotation_job(self, job_id: str) -> Dict:
899891
Returns:
900892
Dict: A dictionary containing the job details.
901893
"""
902-
url = f"{API_URL}/{self.__workspace}/{self.__project_name}/jobs/{job_id}?api_key={self.__api_key}"
903-
response = requests.get(url)
904-
if response.status_code != 200:
905-
try:
906-
error_data = response.json()
907-
if "error" in error_data:
908-
raise RuntimeError(error_data["error"])
909-
raise RuntimeError(response.text)
910-
except ValueError:
911-
raise RuntimeError(f"Failed to get annotation job: {response.text}")
912-
return response.json()
894+
from roboflow.adapters import rfapi
895+
896+
return rfapi.get_annotation_job(self.__api_key, self.__workspace, self.__project_name, job_id)
913897

914898
def create_annotation_job(
915899
self, name: str, batch_id: str, num_images: int, labeler_email: str, reviewer_email: str

roboflow/core/workspace.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -913,13 +913,14 @@ def get_workflow(self, workflow_url):
913913

914914
return rfapi.get_workflow(self.__api_key, self.url, workflow_url)
915915

916-
def create_workflow(self, name, definition=None, description=None):
916+
def create_workflow(self, name, definition=None):
917917
"""Create a new workflow."""
918+
import json
919+
918920
from roboflow.adapters import rfapi
919921

920-
return rfapi.create_workflow(
921-
self.__api_key, self.url, name=name, definition=definition, description=description
922-
)
922+
config = json.dumps(definition) if definition else None
923+
return rfapi.create_workflow(self.__api_key, self.url, name=name, config=config)
923924

924925
# -----------------------------------------------------------------
925926
# Phase 2: Workspace statistics

tests/cli/test_workflow_handler.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ def test_list_no_workspace(self, _mock_resolve):
408408
args = _make_args(workspace=None, api_key=None)
409409
with self.assertRaises(SystemExit) as ctx:
410410
_list_workflows(args)
411-
self.assertEqual(ctx.exception.code, 1)
411+
self.assertEqual(ctx.exception.code, 2)
412412

413413

414414
if __name__ == "__main__":

0 commit comments

Comments
 (0)