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

Talisman hook script ignore talisman upgrade based on variable (#211) #217

Merged
merged 1 commit into from Aug 13, 2020

Conversation

tinamthomas
Copy link
Collaborator

  • set TALISMAN_UPGRADE_TIMEOUT for upgrade timeout(seconds). Default = 10 secs
  • set TALISMAN_SKIP_UPGRADE to true to skip upgrade

@dcRUSTy
Copy link
Collaborator

dcRUSTy commented Aug 6, 2020

https://github.com/thoughtworks/talisman/blob/master/global_install_scripts/talisman_hook_script.bash#L50 this line sends only HTTP HEAD request which is hardly of few KB's what if it succeeds in 10 second and it gets stuck while downloading binary 10MB+

@harinee
Copy link
Collaborator

harinee commented Aug 6, 2020

@tinamthomas I have a few questions and comments about the upgrade timeout:

  1. Have you tested how much time upgrade takes currently, across different environments/network bandwidths/OS? Is a blanket '10 secs' a good benchmark to assume a timeout threshold?
  2. When a timeout occurs, we should show a message to call that out and mention that the user should perform an upgrade manually

@svishwanath-tw
Copy link
Collaborator

The 10 seconds should be used as a timeout to establish connection and nothing else. (ie. --connect-timeout and not --max-timeout).

More information can be found here : https://ec.haxx.se/usingcurl/usingcurl-timeouts.

If HEAD request hangs for 10s. The upgrade should probably be aborted.
The idea is to fail early instead of hang for 1 minute (curl's default) during the HEAD request to figure out the version.

This was a problem reported by a user that worked in a network restricted environment where the connection would never have worked, but the build would hang for every invocation of talisman --scan.

@tinamthomas
Copy link
Collaborator Author

@svishwanath-tw, updated the script to use --connect-timeout. If the connect-timeout exceeds the time, the version will not be fetched and download will be skipped.

@tinamthomas
Copy link
Collaborator Author

@harinee , script updated to print accordingly if update fails, but I think your MR to fix the update should be merged first

…htworks#211)

* set TALISMAN_UPGRADE_TIMEOUT for upgrade timeout(seconds). Default = 10 secs
* set TALISMAN_SKIP_UPGRADE to true to skip upgrade
@svishwanath-tw svishwanath-tw merged commit 440eee6 into thoughtworks:master Aug 13, 2020
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.

Provide option to skip update check in network restricted environments
4 participants