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

question(flaky): cortex -v hangs when checking if there is a newer release? #1268

Closed
freelerobot opened this issue Sep 19, 2024 · 8 comments
Closed
Assignees
Milestone

Comments

@freelerobot
Copy link
Contributor

Flakey issue. Happens 1/3 times.

Steps:

  1. cortex -v or any command really
❯ cortex-nightly -v
v0.5.0-70
  1. Doesn't return, just hangs
  2. After 10 seconds, we get "A new release is available"

What causes this?
Where's our endpoint for checking if there's a more recent version, and is this endpoint prone to lagging?

@freelerobot freelerobot added the P1: important Important feature / fix label Sep 19, 2024
@freelerobot freelerobot added this to the v0.1.0 milestone Sep 19, 2024
@freelerobot freelerobot removed this from the v0.1.0 milestone Sep 19, 2024
@freelerobot freelerobot moved this to Need Investigation in Menlo Sep 19, 2024
@freelerobot freelerobot removed the P1: important Important feature / fix label Sep 19, 2024
@freelerobot freelerobot changed the title question(flaky): why does cortex -v hang when checking if new release is avail? question(flaky): cortex -v hangs when checking if there is a newer release? Sep 19, 2024
@vansangpfiev
Copy link
Contributor

We check the new nightly version with https://delta.jan.ai endpoint. We are using the default timeout (10 seconds).
I think can set the timeout to 1 second, if connection timeouts, just not print any message on version update.

@freelerobot
Copy link
Contributor Author

freelerobot commented Sep 20, 2024

We check the new nightly version with https://delta.jan.ai endpoint. We are using the default timeout (10 seconds). I think can set the timeout to 1 second, if connection timeouts, just not print any message on version update.

Timeout to 1 s is a good workaround!

Q: I'm just wondering whether we need to check versions at the end of every command?

Rationale

  1. Since we're hosting the endpoint ourselves, let's just assume it will be ddos'ed. Then will every cortex command, by default, takes 1 second, returning nothing? How will we build this mini-feature if we were building it for scale & to be antifragile to external dependencies?
  2. Users typically execute many CLI commands in succession, during a single session with Cortex. If they don't want to update initially, then reminding them 10-20 times thereafter, is annoying.
  3. Recall, Cortex is an on-prem, local tool. Many will use it on an intranet. So many external calls out is not our style.

Dumb Ideas

  1. Reserve the version checks for just a few key commands, that users will almost always see:
cortex
cortex -v
cortex start                 # Server start is an impt entrypoint to using cortex, and will call attention.
cortex run                   # Starting an interactive chat shell is another impt entrypoint to using cortex

#Optionally
COMMAND -h            # Not annoying to put it in all the help commands as well

I'm not sure what the best fix is, but I do think shortening timeout to 1sec is a (good) but temporary patch to this issue.

cc @dan-homebrew

@vansangpfiev
Copy link
Contributor

I would like to propose 2 approaches that can help to handle update check better:

  • Approach 1: create a file to save the current version, latest version, last time check the update. This approach is simple, easy to implement, but we need to store our state which can be against our db-less/stateless design.
  • Approach 2: server has a longer lifetime than the CLI, so we can let server checks the update instead of CLI. CLI only need to request local server for the new update information.

cc @dan-homebrew

@freelerobot
Copy link
Contributor Author

freelerobot commented Sep 23, 2024

Sure, open to the above.

  • Approach 1: just add it in the .cortexrc file?

cc @dan-homebrew db-less design holds us back on certain things like this


My only request is that, UX wise, the following should inform users there is an available update:

cortex (-h)
cortex -v

@vansangpfiev
Copy link
Contributor

Will add update info into .cortexrc

checkedForUpdateAt: time
latestRelease: version

Interval to check update:

  • nightly: 10 minutes
  • beta: 1 hour
  • stable: 24 hours
    cc: @dan-homebrew @0xSage

@vansangpfiev vansangpfiev moved this from Need Investigation to In Progress in Menlo Sep 25, 2024
@vansangpfiev vansangpfiev moved this from In Progress to In Review in Menlo Sep 25, 2024
@freelerobot
Copy link
Contributor Author

Will add update info into .cortexrc

checkedForUpdateAt: time
latestRelease: version

Interval to check update:

  • nightly: 10 minutes
  • beta: 1 hour
  • stable: 24 hours
    cc: @dan-homebrew @0xSage

Nice. Is this a cron job that we're always running on the users machine?
Or only run when the next command is executed?

@vansangpfiev
Copy link
Contributor

It only runs when the next CLI command is executed.

@dan-menlo dan-menlo moved this from In Review to Review + QA in Menlo Sep 29, 2024
@freelerobot
Copy link
Contributor Author

Closing as resolved. QA'd adn works really well 🙏

@gabrielle-ong gabrielle-ong added this to the v1.0.0 milestone Oct 3, 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

No branches or pull requests

4 participants