-
Notifications
You must be signed in to change notification settings - Fork 2
Add update notification prompt on command start #136
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| package update | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "time" | ||
|
|
||
| "github.com/localstack/lstk/internal/output" | ||
| "github.com/localstack/lstk/internal/version" | ||
| ) | ||
|
|
||
| type versionFetcher func(ctx context.Context, token string) (string, error) | ||
|
|
||
| type NotifyOptions struct { | ||
| GitHubToken string | ||
| UpdatePrompt bool | ||
| PersistDisable func() error | ||
| } | ||
|
|
||
| const checkTimeout = 500 * time.Millisecond | ||
|
|
||
| func CheckQuietly(ctx context.Context, githubToken string) (current, latest string, available bool) { | ||
| return checkQuietlyWithVersion(ctx, githubToken, version.Version(), fetchLatestVersion) | ||
| } | ||
|
|
||
| func checkQuietlyWithVersion(ctx context.Context, githubToken string, currentVersion string, fetch versionFetcher) (current, latest string, available bool) { | ||
| current = currentVersion | ||
| if current == "dev" { | ||
| return current, "", false | ||
| } | ||
|
|
||
| ctx, cancel := context.WithTimeout(ctx, checkTimeout) | ||
| defer cancel() | ||
|
|
||
| latestVer, err := fetch(ctx, githubToken) | ||
| if err != nil { | ||
| return current, "", false | ||
| } | ||
|
|
||
| if normalizeVersion(current) == normalizeVersion(latestVer) { | ||
| return current, latestVer, false | ||
| } | ||
|
|
||
| return current, latestVer, true | ||
| } | ||
|
|
||
| func NotifyUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions) (exitAfter bool) { | ||
| return notifyUpdateWithVersion(ctx, sink, opts, version.Version(), fetchLatestVersion) | ||
| } | ||
|
|
||
| func notifyUpdateWithVersion(ctx context.Context, sink output.Sink, opts NotifyOptions, currentVersion string, fetch versionFetcher) (exitAfter bool) { | ||
| current, latest, available := checkQuietlyWithVersion(ctx, opts.GitHubToken, currentVersion, fetch) | ||
| if !available { | ||
| return false | ||
| } | ||
|
|
||
| if !opts.UpdatePrompt { | ||
| output.EmitNote(sink, fmt.Sprintf("Update available: %s → %s (run lstk update)", current, latest)) | ||
| return false | ||
| } | ||
|
|
||
| return promptAndUpdate(ctx, sink, opts.GitHubToken, current, latest, opts.PersistDisable) | ||
| } | ||
|
|
||
| func promptAndUpdate(ctx context.Context, sink output.Sink, githubToken string, current, latest string, persistDisable func() error) (exitAfter bool) { | ||
| output.EmitWarning(sink, fmt.Sprintf("Update available: %s → %s", current, latest)) | ||
|
|
||
| responseCh := make(chan output.InputResponse, 1) | ||
| output.EmitUserInputRequest(sink, output.UserInputRequestEvent{ | ||
| Prompt: "A new version is available", | ||
| Options: []output.InputOption{ | ||
| {Key: "u", Label: "Update"}, | ||
| {Key: "s", Label: "SKIP"}, | ||
| {Key: "n", Label: "Never ask again"}, | ||
| }, | ||
| ResponseCh: responseCh, | ||
| }) | ||
|
|
||
| var resp output.InputResponse | ||
| select { | ||
| case resp = <-responseCh: | ||
| case <-ctx.Done(): | ||
| return false | ||
| } | ||
|
|
||
| if resp.Cancelled { | ||
| return false | ||
| } | ||
|
|
||
| switch resp.SelectedKey { | ||
| case "u": | ||
| if err := applyUpdate(ctx, sink, latest, githubToken); err != nil { | ||
| output.EmitWarning(sink, fmt.Sprintf("Update failed: %v", err)) | ||
| return false | ||
| } | ||
| output.EmitSuccess(sink, fmt.Sprintf("Updated to %s — please re-run your command.", latest)) | ||
| return true | ||
| case "n": | ||
| if persistDisable != nil { | ||
| if err := persistDisable(); err != nil { | ||
| output.EmitWarning(sink, fmt.Sprintf("Failed to save preference: %v", err)) | ||
| } | ||
| } | ||
| return false | ||
| default: | ||
| return false | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| package update | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "fmt" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
|
|
||
| "github.com/localstack/lstk/internal/output" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func newTestGitHubServer(t *testing.T, tagName string) *httptest.Server { | ||
| t.Helper() | ||
| return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| resp := githubRelease{TagName: tagName} | ||
| if err := json.NewEncoder(w).Encode(resp); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| })) | ||
| } | ||
|
|
||
| func testFetcher(serverURL string) versionFetcher { | ||
| return func(ctx context.Context, token string) (string, error) { | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, serverURL, nil) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| resp, err := http.DefaultClient.Do(req) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| defer func() { _ = resp.Body.Close() }() | ||
| var release githubRelease | ||
| if err := json.NewDecoder(resp.Body).Decode(&release); err != nil { | ||
| return "", err | ||
| } | ||
| return release.TagName, nil | ||
| } | ||
| } | ||
|
|
||
| func TestCheckQuietlyDevBuild(t *testing.T) { | ||
| current, latest, available := CheckQuietly(context.Background(), "") | ||
| assert.Equal(t, "dev", current) | ||
| assert.Empty(t, latest) | ||
| assert.False(t, available) | ||
| } | ||
|
|
||
| func TestCheckQuietlyNetworkError(t *testing.T) { | ||
| fetch := func(ctx context.Context, token string) (string, error) { | ||
| return "", fmt.Errorf("connection refused") | ||
| } | ||
|
|
||
| current, latest, available := checkQuietlyWithVersion(context.Background(), "", "1.0.0", fetch) | ||
| assert.Equal(t, "1.0.0", current) | ||
| assert.Empty(t, latest) | ||
| assert.False(t, available) | ||
| } | ||
|
|
||
| func TestCheckQuietlyUpdateAvailable(t *testing.T) { | ||
| server := newTestGitHubServer(t, "v2.0.0") | ||
| defer server.Close() | ||
|
|
||
| current, latest, available := checkQuietlyWithVersion(context.Background(), "", "1.0.0", testFetcher(server.URL)) | ||
| assert.Equal(t, "1.0.0", current) | ||
| assert.Equal(t, "v2.0.0", latest) | ||
| assert.True(t, available) | ||
| } | ||
|
|
||
| func TestCheckQuietlyAlreadyUpToDate(t *testing.T) { | ||
| server := newTestGitHubServer(t, "v1.0.0") | ||
| defer server.Close() | ||
|
|
||
| current, latest, available := checkQuietlyWithVersion(context.Background(), "", "v1.0.0", testFetcher(server.URL)) | ||
| assert.Equal(t, "v1.0.0", current) | ||
| assert.Equal(t, "v1.0.0", latest) | ||
| assert.False(t, available) | ||
| } | ||
|
|
||
| func TestNotifyUpdateNoUpdateAvailable(t *testing.T) { | ||
| server := newTestGitHubServer(t, "v1.0.0") | ||
| defer server.Close() | ||
|
|
||
| var events []any | ||
| sink := output.SinkFunc(func(event any) { events = append(events, event) }) | ||
|
|
||
| exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "v1.0.0", testFetcher(server.URL)) | ||
| assert.False(t, exit) | ||
| assert.Empty(t, events) | ||
| } | ||
|
|
||
| func TestNotifyUpdatePromptDisabled(t *testing.T) { | ||
| server := newTestGitHubServer(t, "v2.0.0") | ||
| defer server.Close() | ||
|
|
||
| var events []any | ||
| sink := output.SinkFunc(func(event any) { events = append(events, event) }) | ||
|
|
||
| exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{}, "1.0.0", testFetcher(server.URL)) | ||
| assert.False(t, exit) | ||
| assert.Len(t, events, 1) | ||
| msg, ok := events[0].(output.MessageEvent) | ||
| assert.True(t, ok) | ||
| assert.Equal(t, output.SeverityNote, msg.Severity) | ||
| assert.Contains(t, msg.Text, "Update available") | ||
| } | ||
|
|
||
| func TestNotifyUpdatePromptSkip(t *testing.T) { | ||
| server := newTestGitHubServer(t, "v2.0.0") | ||
| defer server.Close() | ||
|
|
||
| var events []any | ||
| sink := output.SinkFunc(func(event any) { | ||
| events = append(events, event) | ||
| if req, ok := event.(output.UserInputRequestEvent); ok { | ||
| req.ResponseCh <- output.InputResponse{SelectedKey: "s"} | ||
| } | ||
| }) | ||
|
|
||
| exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "1.0.0", testFetcher(server.URL)) | ||
| assert.False(t, exit) | ||
| } | ||
|
|
||
| func TestNotifyUpdatePromptNever(t *testing.T) { | ||
| server := newTestGitHubServer(t, "v2.0.0") | ||
| defer server.Close() | ||
|
|
||
| persistCalled := false | ||
|
|
||
| var events []any | ||
| sink := output.SinkFunc(func(event any) { | ||
| events = append(events, event) | ||
| if req, ok := event.(output.UserInputRequestEvent); ok { | ||
| req.ResponseCh <- output.InputResponse{SelectedKey: "n"} | ||
| } | ||
| }) | ||
|
|
||
| exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{ | ||
| UpdatePrompt: true, | ||
| PersistDisable: func() error { | ||
| persistCalled = true | ||
| return nil | ||
| }, | ||
| }, "1.0.0", testFetcher(server.URL)) | ||
| assert.False(t, exit) | ||
| assert.True(t, persistCalled) | ||
| } | ||
|
|
||
| func TestNotifyUpdatePromptCancelled(t *testing.T) { | ||
| server := newTestGitHubServer(t, "v2.0.0") | ||
| defer server.Close() | ||
|
|
||
| var events []any | ||
| sink := output.SinkFunc(func(event any) { | ||
| events = append(events, event) | ||
| if req, ok := event.(output.UserInputRequestEvent); ok { | ||
| req.ResponseCh <- output.InputResponse{Cancelled: true} | ||
| } | ||
| }) | ||
|
|
||
| exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "1.0.0", testFetcher(server.URL)) | ||
| assert.False(t, exit) | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: what about adding integration tests? It should be possible if we make the github URL overwritable via env var, right? |
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
thought: we're adding latency to all commands with that check, so I am wondering if we should do the check asynchronously or just less often in the future (say once per 24h). Just a note for the future, no action required.
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.
Created an issue already for this after noticing. What other tools do is do the check in the background, write it down and on next start show it.
For now I've set the timeout to 500ms. If it fails because of the timeout we won't show it.