-
Notifications
You must be signed in to change notification settings - Fork 18
Conversation
This commit adds a new update subcommand that queries a Coder instance for its current version, fetches the corresponding version from GitHub releases if required, and updates the binary in-place.
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.
Tested locally and works wonderfully!
executablePath string | ||
fs afero.Fs | ||
httpClient getter | ||
osF func() string |
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.
osF can just be a string instead of a function.
Get(url string) (*http.Response, error) | ||
} | ||
|
||
func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versionArg string) error { |
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.
This function could benefit from being split up a little bit more, it's fairly long as it stands right now
Also I think for the initial version we shouldn't worry about querying github API, the release artifacts are consistent right now. This PR is getting pretty big |
I found the opposite, we have |
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.
LGTM, my comments are minor, feel free to address them in a follow-up (or not at all).
} | ||
|
||
for _, prefix := range pathBlockList { | ||
if HasFilePathPrefix(u.executablePath, prefix) { |
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.
This behavior is good for now, just wanted to leave a suggestion for your consideration in a future iteration: we could also write the file to a well-known location (e.g. ~/.local/coder/bin
) or similar, and then use LookPath to see if we find the file we just wrote. If the version doesn't match, then we know that the user has something else earlier on their path obscuring the location of where we wrote the file, so we could issue a message in that case instructing users what to do (e.g. move ~/.local/coder/bin earlier in the PATH, or remove the other one from the PATH, etc)
Version string `json:"version"` | ||
}{} | ||
|
||
body, err := ioutil.ReadAll(resp.Body) |
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.
Ah, we should consider increasing our version requirement for coder-cli to 1.16 so that we can use io.ReadAll instead (ioutil stuff is now deprecated). No rush to do that, though, we can do it after 1.17 is released (should be soon)
This reverts commit 0e63d2f.
This commit adds a new update subcommand that queries a Coder instance for its current version, fetches the corresponding version from GitHub releases if required, and updates the binary in-place.
Currently, we only check Major.Minor.Patch version, and not pre-release. (Edit: checking pre-release now)
This will obviously not work if the corresponding release of coder-cli is not yet available.