Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: cortex update stdouts "Update cortex sucessfully" even when update failed #1257

Closed
1 of 6 tasks
Tracked by #1087
freelerobot opened this issue Sep 19, 2024 · 3 comments
Closed
1 of 6 tasks
Tracked by #1087
Assignees
Labels
category: app shell Installer, updaters, distributions good first issue Good for newcomers type: bug Something isn't working
Milestone

Comments

@freelerobot
Copy link
Contributor

freelerobot commented Sep 19, 2024

Cortex version

v65->v67

Describe the Bug

  1. User runs cortex update
  2. For some reason update fails (cut wifi halfway), we still log "Update cortex successfully"
  3. Expected: stdout that the update failed, missing permissions, etc.
❯ cortex-nightly -v
v0.5.0-65

A new release of cortex is available: v0.5.0-65 -> v0.5.0-67
To upgrade, run: cortex-nightly update
❯ cortex-nightly update
Server is running. Stopping server before updating!
HTTP error: Failed to read connection
Validating download items, please wait..
Start downloading: cortex-nightly.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 11.6M  100 11.6M    0     0  41.8M      0 --:--:-- --:--:-- --:--:-- 42.0M
Update cortex sucessfully
❯ cortex-nightly
libc++abi: terminating due to uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in create_directories: Permission denied ["/Users/nicolezhu/cortexcpp-nightly/models"]
[1]    10434 abort      cortex-nightly

How I fixed it:

  1. I just ran sudo cortex update
  2. And everything worked fine
❯ sudo cortex-nightly update
Password:
Validating download items, please wait..
Start downloading: cortex-nightly.tar.gz
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 11.6M  100 11.6M    0     0  40.2M      0 --:--:-- --:--:-- --:--:-- 40.3M
Update cortex sucessfully
❯ cortex-nightly
Cortex.cpp CLI
cortex-nightly [OPTIONS] [SUBCOMMAND]

Options:
  -h,--help                   Print this help message and exit
  --verbose                   Verbose logging

Steps to Reproduce

No response

Screenshots / Logs

No response

What is your OS?

  • MacOS
  • Windows
  • Linux

What engine are you running?

  • cortex.llamacpp (default)
  • cortex.tensorrt-llm (Nvidia GPUs)
  • cortex.onnx (NPUs, DirectML)
@freelerobot freelerobot added the type: bug Something isn't working label Sep 19, 2024
@freelerobot freelerobot added category: app shell Installer, updaters, distributions good first issue Good for newcomers labels Sep 19, 2024
@freelerobot freelerobot added this to the v0.1.0 milestone Sep 19, 2024
@freelerobot freelerobot moved this to Need Investigation in Menlo Sep 19, 2024
@freelerobot freelerobot changed the title bug: cortex update logs "Update cortex sucessfully" when it failed bug: cortex update logs "Update cortex sucessfully" even when update failed Sep 19, 2024
@hiento09
Copy link
Contributor

Add @vansangpfiev here to help modify the update message to the user. Change from cortex-nightly update to sudo cortex-nightly update since cortex-nightly is in /usr/local/bin, which requires root permission to replace.

@freelerobot freelerobot changed the title bug: cortex update logs "Update cortex sucessfully" even when update failed bug: cortex update stdouts "Update cortex sucessfully" even when update failed Sep 19, 2024
@freelerobot
Copy link
Contributor Author

Add @vansangpfiev here to help modify the update message to the user. Change from cortex-nightly update to sudo cortex-nightly update since cortex-nightly is in /usr/local/bin, which requires root permission to replace.

The core issue is that updates can fail (for various reasons) but we still stdout to users that it was successful. We need better error handling here.

@freelerobot
Copy link
Contributor Author

Closing as we fixed related upstream issue

@github-project-automation github-project-automation bot moved this from Need Investigation to Completed in Menlo Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: app shell Installer, updaters, distributions good first issue Good for newcomers type: bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

3 participants