Add unified ul CLI for Platform and YOLO - #59
Conversation
|
👋 Hello @JaviChulvi, thank you for submitting a
For more guidance, please refer to our Contributing Guide. Don't hesitate to leave a comment if you have any questions. Thank you for contributing to Ultralytics! 🚀 |
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review
Made with ❤️ by Ultralytics Actions
Reviewed the unified CLI runtime, generated copy, authentication provider, CI/configuration changes, and focused tests. The main command flow is coherent, but empty-value parsing can turn help into an unintended mutation and credential persistence needs atomic, restrictive file handling.
💬 Posted 2 inline comments
⚠️ HIGHauth.py:51This truncates the shared settings file in place and creates a new file with the process umask (commonly world-readable under a 022 umask). A freshul logincan therefore expose the bearer API key, while interruption or concurrent YOLO settings updates can leave the entire settings file truncated or lose unrelated changes. Write a same-directory temporary file with restrictive permissions and atomically replace the target, then regenerate the package copy.- 💡 MEDIUM
cli.py:78An empty assignment consumes a following help token:ul cloud projects create project=p name= --helpbecomesname="--help", sohelp_requestedstays false and the write operation is sent instead of displaying help. Excludehelp,--help, and-hfrom this empty-value join; regenerate the installed copy afterward.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 2
Made with ❤️ by Ultralytics Actions
Since the previous review, the empty-assignment help regression is fixed, and credential writes now use restrictive temporary files with atomic replacement. One race remains: concurrent settings writers can still lose unrelated or newer settings during the unsynchronized read-modify-replace sequence. No other concrete issues found.
💬 Posted 1 inline comment
- 💡 MEDIUM
auth.py:45The temporary-file replacement is atomic, but this read-modify-write is still vulnerable to lost updates. Ifyoloor anotherul login/logoutupdatessettings.jsonafter this read and beforeos.replace, this stale dictionary overwrites the newer credential or unrelated settings such asruns_dir. Serialize the read/merge/replace with a cross-process lock (or the shared settings writer's lock), then regenerate the packaged copy.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 3
Made with ❤️ by Ultralytics Actions
Since the previous review, the empty-value/help regression remains fixed, and the credential-write race is resolved by removing the SDK settings writer. The current diff has one remaining behavioral issue: project owners are still implicitly inferred even though the CLI contract explicitly forbids that. Otherwise, no additional concrete regressions found.
💬 Posted 1 inline comment
- 💡 MEDIUM
cli.py:234This infersownerfor every positional owner operation, includingprojects. As a result,ul cloud projectsandul cloud projects project=pperform an account lookup and target the logged-in username, despite the documented contract that project owners are never inferred. Make owner inference policy-aware and do not exemptprojects' required owner from the missing-argument check; regenerate the packaged copy.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 4
Made with ❤️ by Ultralytics Actions
Since review 3, the credential-writer race and help-token regression remain addressed, and the owner-inference clarification is reflected in the current contract. I found one remaining bounded parsing defect: nested non-finite JSON values are accepted by the CLI and fail only later during request serialization. All other reviewed changes appear consistent with the stated design.
💬 Posted 1 inline comment
- 📝 LOW
cli.py:131json.loadsaccepts non-standardNaN,Infinity, and-Infinityvalues inside objects and arrays. For example,body={"conf":NaN}passes the top-level type check and reaches the SDK as a non-finite nested value, causing invalid JSON or a later serialization failure instead of the documented local input error before any request. Reject these constants while parsing (or recursively validate finiteness), then regenerate the packaged copy.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 5
Made with ❤️ by Ultralytics Actions
Since review 4, the nested non-finite-number parsing fix is present, and the earlier authentication race, help-token, and owner-inference issues remain addressed. The unified CLI and generated copy otherwise look consistent; one low-severity API error-message regression remains because standard detail responses are not displayed.
💬 Posted 1 inline comment
- 📝 LOW
cli.py:280When the API returns the standard SDK error shape{"detail": "invalid"}(as covered bytests/test_python_sdk.py),body.get("error")isNone, so the CLI prints onlyAPI request failed (HTTP 422).and drops the server message. Read thedetailfield as a fallback as well.
| return 0 | ||
| except APIError as error: | ||
| body = error.json | ||
| detail = body.get("error") if isinstance(body, dict) else None |
There was a problem hiding this comment.
I'd keep the current implementation here. The Platform's error contract defines {"error": "..."}, which the CLI already displays.
The detail fixture tests whether the SDK preserves an arbitrary error response body; it doesn't establish the Platform's response format. The shared API validator and prediction routes also return error.
Unless we can identify an actual Platform endpoint returning detail, this would add a fallback without a demonstrated API case.
| return 0 | ||
| except APIError as error: | ||
| body = error.json | ||
| detail = body.get("error") if isinstance(body, dict) else None |
There was a problem hiding this comment.
I'd keep the current implementation here. The Platform's error contract defines {"error": "..."}, which the CLI already displays.
The detail fixture tests whether the SDK preserves an arbitrary error response body; it doesn't establish the Platform's response format. The shared API validator and prediction routes also return error.
Unless we can identify an actual Platform endpoint returning detail, this would add a fallback without a demonstrated API case.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 6
Made with ❤️ by Ultralytics Actions
Since review 5, the prior parsing and authentication findings remain addressed, and the API error-detail concern was settled by the author. The current diff otherwise looks consistent with the generated SDK design; one new regression remains: the documented no-argument ul login path crashes instead of prompting. പരിഗ
💬 Posted 1 inline comment
- 💡 MEDIUM
cli.py:274The documentedul logininvocation is delegated unchanged as['login'], so no API key is supplied and this code does not prompt for one. The installedultralyticshandler accessesargs[1], causing an uncaughtIndexErrorinstead of prompting or returning a controlled CLI error. Read the key here before delegation, or change the documentation to requireul login API_KEY.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 7
Made with ❤️ by Ultralytics Actions
Reviewed all 12 changed files, including the unified CLI, generated package copy, packaging/CI configuration, documentation, and tests. The documented ul login API_KEY path is consistent with the current delegation, and no additional concrete security, correctness, compatibility, or performance issues remain. LGTM.
Why
Add
ulas one entry point for Platform API commands and existing YOLO commands, starting with the resource commands in the unified CLI plan. Cloud commands work with the SDK alone; local commands and login/logout use the installedultralyticspackage. Higher-level cloud workflows can follow separately.Example: login → train → deploy → predict
From this PR's checkout, install into a Python 3.11+ environment.
ultralyticssupplies login/logout and local YOLO commands;ul cloudcan also run with the SDK alone usingULTRALYTICS_API_KEY.Copy the model creation response's
idintoMODEL_IDbelow.model_ididentifies the destination model record;train_args.modelselects the starting weights. This small example usescoco8.yaml; a Platform dataset can instead be passed asul://YOUR_USERNAME/datasets/YOUR_DATASET.Cloud training spends credits. These commands submit or inspect one operation and exit; they do not wait for the whole workflow to finish.
ul cloud training gpu-availability ul cloud training start model_id=MODEL_ID gpu_type=rtx-4090 \ 'train_args={"model":"yolo26n.pt","data":"coco8.yaml","epochs":1,"imgsz":640,"batch":16}' # Repeat this check until model.status is "completed" and model.hasWeights is true. # If training fails or is cancelled, inspect the response before continuing. ul cloud models retrieve project=cli-demo model=detectorAfter training completes, create the endpoint:
ul cloud deployments create project=cli-demo model=detector \ deployment=cli-demo-api name="CLI demo API" region=europe-west1 # Repeat until deployment.status is "ready" before sending predictions. ul cloud deployments retrieve deployment=cli-demo-api ul cloud deployments health deployment=cli-demo-api # Use an existing local image.jpg. The @ prefix opens the file for multipart upload. ul cloud deployments predict deployment=cli-demo-api \ 'body={"file":"@image.jpg","conf":0.25}'Path
ownerdefaults to the logged-in username; passowner=WORKSPACEexplicitly when accessing another workspace. Nested request bodies use JSON, and argument names match the SDK's Python names. Useul cloud <resource> <operation> --helpfor the full signature.Example: YOLO compatibility
Local commands delegate to the existing
ultralytics.cfg.entrypointwith the same arguments and YOLO executable identity. YOLO continues to own argument validation, defaults, execution, and settings.yolo train model=yolo26n.pt data=coco8.yaml epochs=1ul train model=yolo26n.pt data=coco8.yaml epochs=1yolo val model=yolo26n.pt data=coco8.yamlul val model=yolo26n.pt data=coco8.yamlyolo predict model=yolo26n.pt source=image.jpgul predict model=yolo26n.pt source=image.jpgyolo export model=yolo26n.pt format=onnxul export model=yolo26n.pt format=onnxyolo track model=yolo26n.pt source=video.mp4ul track model=yolo26n.pt source=video.mp4yolo benchmark model=yolo26n.pt data=coco8.yaml format=onnxul benchmark model=yolo26n.pt data=coco8.yaml format=onnxTask prefixes (
detect,segment,semantic,depth,classify,pose,obb) and utilities (settings,checks,cfg,copy-cfg,solutions,login,logout) follow the same forwarding path. Privateul://model and dataset inputs continue through YOLO's existing loaders.ul trainruns training locally;ul cloud training startlaunches cloud training. Top-levelul helpshows unified help, andul versionreports both SDK and YOLO versions.Design
The CLI reads argument types and help from SDK signatures and docstrings, then calls the SDK to make requests. This keeps API definitions and transport in their existing owner and avoids maintaining a second command registry. YOLO continues to own local commands and credential writes.
Root
cli.pyis the maintained source. Its generated copy is committed because package builds and CI consumesdk/python. Only multipart file fields need a small generated mapping.Validation: Python 3.11/3.14 tests, real loopback HTTP, a Git-subdirectory package install, Ruff, and regeneration checks pass locally.
Before merge: openapi#50 must land first; current CI fails because it regenerates from generator
main. Login and launcher documentation still needs updating before release.