Repository navigation
fix(tui): keep session status current in the app - #45
Merged
Merged
Conversation
A refresh carried the last status read onto the new list for display, then ran the new read on that carried copy. The read only covers sessions in plain running state, so every session last seen idle or working was skipped from then on and its status froze: a finished setup kept showing as running, manual refresh included. The read now gets the list as the cloud returned it. The app also refreshes on its own while a setup runs and every 30 seconds otherwise, since sessions go idle, work and park themselves, and a failed read no longer ends the refreshing.
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR do and why?
The app's session status could freeze. A refresh carries the last status read onto the new list so rows don't blink, then ran the new status read on that carried copy. The read only covers sessions in plain
runningstate, so a session last seen idle or working was skipped on every later refresh, manual ones included. Its state, strain and setup status stayed at whatever was read first, such as a finished setup still showing· setup. The read now gets the list as the cloud returned it, and the carried copy is used only for display.The app also refreshed on its own only while a create, delete, fill or bake was underway. It now also polls quickly while a setup runs, and every 30 seconds otherwise, because sessions go idle, work and park themselves with no local signal. A failed read no longer ends the refreshing.
Related issue
None.
Tests
TestRefreshReadsStatusFresh: the carried reading stays on screen, and the new status read gets the cloud's list. It fails without the fix (got idle/"running").go test -race ./...,go vet ./...andgofmt -lpass.Checklist
gofmt -l .produces no output andgo vet ./...passesmake buildsucceeds (if this touches supervisor/session code)