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

epic: Separate CLI and API processes #1415

Closed
2 tasks done
gabrielle-ong opened this issue Oct 3, 2024 · 12 comments · Fixed by #1499
Closed
2 tasks done

epic: Separate CLI and API processes #1415

gabrielle-ong opened this issue Oct 3, 2024 · 12 comments · Fixed by #1499
Assignees
Milestone

Comments

@gabrielle-ong
Copy link
Contributor

gabrielle-ong commented Oct 3, 2024

Goal

  • Cortex will primarily be an OpenAI API equivalent server
  • Cortex CLI should be a layer on top of the API Server
  • CLI and server are now two distinct binaries

Subtasks

Discussion:

#1386

Success Criteria (windows, max, linux)

  • Installer: Installer installs 2 different binaries
  • Uninstaller: Uninstalls 2 different binaries
  • Cortex update replaces the app, installer, uninstaller and binary file (without installing cortex.llamacpp)
  • all CLI commands will start API server, if not running
  • Edge case: User run cortex update on v1.0.0 → prompted to install cortex-server as with 1.0.1
@gabrielle-ong gabrielle-ong converted this from a draft issue Oct 3, 2024
@github-project-automation github-project-automation bot moved this to Requested in Models Oct 3, 2024
@gabrielle-ong gabrielle-ong removed this from Models Oct 3, 2024
@dan-menlo dan-menlo added this to the v1.0.1 milestone Oct 14, 2024
@hiento09 hiento09 self-assigned this Oct 14, 2024
@hiento09
Copy link
Contributor

hiento09 commented Oct 14, 2024

Hey guys @dan-homebrew @vansangpfiev @gabrielle-ong @namchuai @nguyenhoangthuan99 @0xSage , I’d like to present an idea for the installer and updater when we separate the CLI and server into two distinct binaries, as follows:

Given that:

In the current updater, the user runs the cortex update command, which causes the cortex process to download the new binary version and replace the existing binary. However, other components of the installer, such as the uninstaller and post-uninstall script, are not updated because we only replace the binary file, not the entire app version on the user's machine.

Solution:

Instead of the current cortex update only downloading and replacing the binary file, the cortex update command will pull the installer file and install it in headless mode using the following commands:

  • macos: touch /var/tmp/cortex_installer_skip_postinstall && sudo installer -pkg cortexcpp-nightly-network.pkg -target / && rm /var/tmp/cortex_installer_skip_postinstall
  • Linux: echo -e "n\n" | sudo SKIP_POSTINSTALL=true apt install -y --allow-downgrades ./cortexcpp-nightly-network.deb
  • Windows:
    • Powershell Start-Process -FilePath ".\cortex-0.5.0-154-windows-amd64-network-installer.exe" -ArgumentList "/VERYSILENT /SUPPRESSMSGBOXES /NORESTART /SkipPostInstall" -Wait
    • CMD start /wait "" network-setup.exe /VERYSILENT /SUPPRESSMSGBOXES /NORESTART /SkipPostInstall
      In these commands, we skip the postinstall process, which installs the cortex.llamacpp engine. This allows the installer to replace only the app, installer, uninstaller, and the new binary file. With this approach, the Cortex code no longer needs to handle the complexity of replacing two binaries; it can simply run the commands above to update both binaries and the scripts in the installer and uninstaller simultaneously.

What do you guys think of this idea? Please leave your comments. Thanks!

@vansangpfiev
Copy link
Contributor

Hey guys @dan-homebrew @vansangpfiev @gabrielle-ong @namchuai @nguyenhoangthuan99 @0xSage , I’d like to present an idea for the installer and updater when we separate the CLI and server into two distinct binaries, as follows:

Given that:

In the current updater, the user runs the cortex update command, which causes the cortex process to download the new binary version and replace the existing binary. However, other components of the installer, such as the uninstaller and post-uninstall script, are not updated because we only replace the binary file, not the entire app version on the user's machine.

Solution:

Instead of the current cortex update only downloading and replacing the binary file, the cortex update command will pull the installer file and install it in headless mode using the following commands:

  • macos: SKIP_POSTINSTALL=true && sudo installer -pkg cortex-1.0.0-rc1-mac-universal-network-installer.pkg -target /
  • Linux: echo -e "n\n" | SKIP_POSTINSTALL=true && sudo apt install -y --allow-downgrades ./cortex-1.0.0-165-linux-amd64-network-installer.deb
  • Windows: Start-Process -FilePath ".\cortex-0.5.0-154-windows-amd64-network-installer.exe" -ArgumentList "/SkipPostInstall /VERYSILENT /SUPPRESSMSGBOXES /NORESTART" -Wait

In these commands, we skip the postinstall process, which installs the cortex.llamacpp engine. This allows the installer to replace only the app, installer, uninstaller, and the new binary file. With this approach, the Cortex code no longer needs to handle the complexity of replacing two binaries; it can simply run the commands above to update both binaries and the scripts in the installer and uninstaller simultaneously.

What do you guys think of this idea? Please leave your comments. Thanks!

I totally agree with the above solution. That way, we won't need to change much of the updater code in the long-term.
While calling an installer from cortex can be tricky, it is doable.

@freelerobot
Copy link
Contributor

freelerobot commented Oct 15, 2024 via email

@hiento09
Copy link
Contributor

Largely sounds good. From the user's perspective, what is the experience like? - Do new windows/views pop up (if so, then no go) - Is there an increased update wait time?

On Tue, Oct 15, 2024 at 7:35 AM vansangpfiev @.> wrote: Hey guys @dan-homebrew https://github.com/dan-homebrew @vansangpfiev https://github.com/vansangpfiev @gabrielle-ong https://github.com/gabrielle-ong @namchuai https://github.com/namchuai @nguyenhoangthuan99 https://github.com/nguyenhoangthuan99 @0xSage https://github.com/0xSage , I’d like to present an idea for the installer and updater when we separate the CLI and server into two distinct binaries, as follows: Given that: In the current updater, the user runs the cortex update command, which causes the cortex process to download the new binary version and replace the existing binary. However, other components of the installer, such as the uninstaller and post-uninstall script, are not updated because we only replace the binary file, not the entire app version on the user's machine. Solution: Instead of the current cortex update only downloading and replacing the binary file, the cortex update command will pull the installer file and install it in headless mode using the following commands: - macos: SKIP_POSTINSTALL=true && sudo installer -pkg cortex-1.0.0-rc1-mac-universal-network-installer.pkg -target / - Linux: echo -e "n\n" | SKIP_POSTINSTALL=true && sudo apt install -y --allow-downgrades ./cortex-1.0.0-165-linux-amd64-network-installer.deb - Windows: Start-Process -FilePath ".\cortex-0.5.0-154-windows-amd64-network-installer.exe" -ArgumentList "/SkipPostInstall /VERYSILENT /SUPPRESSMSGBOXES /NORESTART" -Wait In these commands, we skip the postinstall process, which installs the cortex.llamacpp engine. This allows the installer to replace only the app, installer, uninstaller, and the new binary file. With this approach, the Cortex code no longer needs to handle the complexity of replacing two binaries; it can simply run the commands above to update both binaries and the scripts in the installer and uninstaller simultaneously. What do you guys think of this idea? Please leave your comments. Thanks! I totally agree with the above solution. That way, we won't need to change much of the updater code in the long-term. While calling an installer from cortex can be tricky, it is doable. — Reply to this email directly, view it on GitHub <#1415 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQVWFCCBRX42LWHW2W7JDYDZ3RIL7AVCNFSM6AAAAABPJ5KC3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJSGUZDCMZRGM . You are receiving this because you were mentioned.Message ID: @.>

@0xSage

  • Do new windows/views pop up (if so, then no go)? => No, all the commands above run the installer in headless mode. I tested them, and they work as expected.
  • Is there an increased update wait time? => We use a network installation but disable the post-install script that pulls cortex.llamacpp from the internet. Instead, it only pulls the new binaries, which are less than 30MB. I tested this, and the update process is smooth and quick.

@freelerobot
Copy link
Contributor

freelerobot commented Oct 15, 2024 via email

@hiento09
Copy link
Contributor

hiento09 commented Oct 15, 2024

Hi @dan-homebrew @0xSage , we need to finalize the binary file name of the cortex server by tomorrow so that @vansangpfiev and I can proceed with splitting the binary and updating the installer. Could you help us finalize it? Thanks!

My proposal:

  • cortex-server-nightly
  • cortex-server-beta
  • cortex-server

@vansangpfiev
Copy link
Contributor

We also need to handle the case that user using v1.0.0 update cortex via cortex update command.
I would like to propose an approach:

  • User run cortex update on v1.0.0 → only cortex binary (which is CLI in v1.0.1) is updated
  • When user run next command, cortex CLI checks if cortex-server exists, if not, output message to ask user to download cortex-server. Something like: cortex v1.0.1 requires cortex-server, to install, run: cortex update --server

cc: @dan-homebrew @gabrielle-ong @hiento09

@gabrielle-ong
Copy link
Contributor Author

Success Criteria (windows, max, linux)

  • Installer: Installer installs 2 different binaries
  • Uninstaller: Uninstalls 2 different binaries
  • Cortex update replaces the app, installer, uninstaller and binary file (without installing cortex.llamacpp)
  • all CLI commands will start API server, if not running
  • Edge case: User run cortex update on v1.0.0 → prompted to install cortex-server as with 1.0.1

@gabrielle-ong gabrielle-ong modified the milestones: v1.0.2, v1.0.1 Oct 21, 2024
@gabrielle-ong
Copy link
Contributor Author

gabrielle-ong commented Oct 21, 2024

@hiento09 and @vansangpfiev
Can I check if anything missing from this success criteria for the Separate CLI & API epic?

Success Criteria (windows, max, linux)

Installer: Installer installs 2 different binaries
Uninstaller: Uninstalls 2 different binaries
Cortex update replaces the app, installer, uninstaller and binary file (without installing cortex.llamacpp)
all CLI commands will start API server, if not running
Edge case: User run cortex update on v1.0.0 → prompted to install cortex-server as with 1.0.1

@vansangpfiev
Copy link
Contributor

@gabrielle-ong

all CLI commands will start API server, if not running

cortex pull and cortex engines install do not need to start API server for now
Others Success Criteria are correct. @hiento09 Please help to double check.

@hiento09
Copy link
Contributor

@gabrielle-ong The installer and uninstaller meet the success criteria. Sang and I worked on the same PR here #1499 . You can test it from the nightly version 183 where our PR was merged.
image

@gabrielle-ong
Copy link
Contributor Author

Thanks @hiento09 and @vansangpfiev, marking as complete
Tracking the followup task #1523 (cortex pull and cortex engines to use API server)

Successfully installed mac, windows, linux
Sucessfully uninstalled cortex-nightly and cortex-server-nightlyEdge case successfully encountered last week, rancortex update --server`

@gabrielle-ong gabrielle-ong moved this from Review + QA to Completed in Menlo Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants