fix: log model download URL and destination - #1564
Conversation
Signed-off-by: RSKKSOFFICIAL <rsksofficial02@gmail.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds optional, user-visible logging to the model download flow so recognize:download-models prints the archive URL and relevant filesystem paths (Fixes #1403).
Changes:
- Extend
DownloadModelsService::download()to accept an optional logger callback. - Emit log messages for model directory removal, archive download URL/path, and extraction path.
- Wire the callback to
occ recognize:download-modelsconsole output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lib/Service/DownloadModelsService.php | Adds optional callback-based logging around model download/extract steps. |
| lib/Command/DownloadModels.php | Passes a console logger callback so messages are printed during the CLI command. |
馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * @return void | ||
| * @throws \Exception | ||
| */ | ||
| public function download() : void { | ||
| public function download(?callable $log = null) : void { | ||
| $log ??= static function (string $message): void { | ||
| }; |
There was a problem hiding this comment.
Thanks @marcelklehr! Added the @PARAM annotation for $log to the docblock.
|
Thank you! 馃挋 |
Summary
Fixes #1403.
The
recognize:download-modelscommand previously did not show the model download URL and destination path.This change adds optional logging to
DownloadModelsService::download()and wires it to the console command output.The command now reports:
This makes it possible to identify the exact URL and paths when the Nextcloud instance does not have direct Internet access.
Testing
Tested locally with:
docker compose exec --user www-data nextcloud php occ recognize:download-models