-
Notifications
You must be signed in to change notification settings - Fork 59
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
Track CLI version. #257
Track CLI version. #257
Conversation
The implementation currently doesn't work. Discussed with Kailan and am addressing a different way. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lightly tested but LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code reviewed and tested locally – works as expected on configurations from 0.26.x. No more requests than needed are made to the config service. Ship it 🚀
Problem: When we have an updated configuration the CLI code is updated so it can parse the new configuration schema, but if the CLI is run (this initial run will cause the config data to be cached for 5mins) before the binary is updated to a new version (which includes code that expects new config fields) and if the new CLI version is then subsequently run before the currently cached config TTL has expired, then a user will find the new CLI code will try to read a struct field that doesn't have a value because the underlying configuration data isn't updated.
See also: #258
Solution: The customer waits 5 minutes (or however long the TTL is set to in the config) and then tries the CLI command again and the configuration will be considered stale and subsequently updated before the subcommand is executed.
Better Solution: Implement a check alongside the 'stale config check' that validates if the CLI binary was recently modified (i.e. updated) and if so we'll force the fetching of the configuration.
Notes: this will require an update to the configuration endpoint to include a new
version
.