Skip to content

Add timeout to pypi version request, preventing undue hangups on startup#855

Merged
maartenbreddels merged 3 commits intowidgetti:masterfrom
ntjess:add-version-timeout
Dec 13, 2024
Merged

Add timeout to pypi version request, preventing undue hangups on startup#855
maartenbreddels merged 3 commits intowidgetti:masterfrom
ntjess:add-version-timeout

Conversation

@ntjess
Copy link
Copy Markdown
Contributor

@ntjess ntjess commented Nov 8, 2024

Fixes #853

@maartenbreddels
Copy link
Copy Markdown
Contributor

We thought a bit about it. What do you think of this solution? Where we only show it if we found a new version on time.

@ntjess
Copy link
Copy Markdown
Contributor Author

ntjess commented Nov 12, 2024

Have you tested the timing between the browser opening and the message being printed in the console?

My only concern with this new approach is the following scenario:

  • User runs solara run ...
  • Browser window opens, user no longer thinks about their terminal
  • Upgrade message prints, but user never sees it because their focus is already on the browser
  • App generates print() / logging statements that obfuscate the upgrade message
  • User never learns there is a solara update

@ntjess
Copy link
Copy Markdown
Contributor Author

ntjess commented Nov 12, 2024

Another thought: We may also want to read the pip conf (through an approach like pip config list -> check if index-url is overwritten) to see which URL should be hit instead of defaulting to pypi.

This is a more robust solution than just a timeout, and would work for e.g. private pypi setups like artifactory. However, it is nontrivial to know ahead of time where pip will fetch from (overloads at global/user level, even per-pip command). So maybe the complexity outweighs the benefits.

@maartenbreddels
Copy link
Copy Markdown
Contributor

I revered to your original idea, but we now execute it in a thread, so it is never delaying on the terminal. We still add a timeout, so the thread doesn't hang around (500ms should be enough, overwise indeed, focus is probably shifted).

Using a Lock/mutex, we make sure the printing of text does not mix.

@maartenbreddels maartenbreddels merged commit acfb977 into widgetti:master Dec 13, 2024
@maartenbreddels
Copy link
Copy Markdown
Contributor

Thanks @ntjess !

@ntjess ntjess deleted the add-version-timeout branch January 5, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a short timeout to _check_version() requests call

2 participants