Allow specifying message to end the spinner with #5972
Conversation
✅ Deploy Preview for odo-docusaurus-preview canceled.
|
There was a problem hiding this comment.
Added a comment. I think #5964 is also trying to change how the "Executing the application..." message is displayed to the users.
But regarding the original issue addressed here, would it make sense to capture the interrupt signal in kubeexec.go (where the command goroutine is spawned), and then, in the goroutine itself, detect that signal and call the handler in a way that allows it to handle this case differently (maybe with a status different from Stopped/Errored)?
This way, I think, it would solve the issue of notifying users if the command errors out anytime after it has been started, and it would not display any message if the dev session stops normally.
What do you think?
| case remotecmd.Stopped, remotecmd.Errored: | ||
| s.End(status == remotecmd.Stopped) |
There was a problem hiding this comment.
If there is an error a few seconds/minutes after the command has started, users would never be aware of this, I think. See my comment in #5964 (review)
For certain long running tasks, we need to specify a message that doesn't confuse the user when the spinnner ends. This commit adds `EndWithStatus` which is a wrapper on top of `End` and lets the developer specify a custom message to end the spinner with. Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>
@rm3l I had a chat about this with @feloy. His point was that the status indicated by odo is technically correct. So we concluded to change the wording so that it's less confusing for the user. I have pushed a change accordingly. PTAL and let me know what you think about it. |
Co-authored-by: Armel Soro <armel@rm3l.org>
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rm3l The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@dharmit This PR is no longer about using |
|
Kudos, SonarCloud Quality Gate passed!
|
|
/override windows-integration-test/Windows-test |
|
@dharmit: Overrode contexts on behalf of dharmit: windows-integration-test/Windows-test In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
…loper#5972)" This reverts commit 72d632b.
* Allow specifying message to end the spinner with For certain long running tasks, we need to specify a message that doesn't confuse the user when the spinnner ends. This commit adds `EndWithStatus` which is a wrapper on top of `End` and lets the developer specify a custom message to end the spinner with. Signed-off-by: Dharmit Shah <shahdharmit@gmail.com> * Update pkg/devfile/adapters/kubernetes/component/commandhandler.go Co-authored-by: Armel Soro <armel@rm3l.org> Signed-off-by: Dharmit Shah <shahdharmit@gmail.com> Co-authored-by: Armel Soro <armel@rm3l.org>
Signed-off-by: Dharmit Shah shahdharmit@gmail.com
What type of PR is this:
/kind bug
What does this PR do / why we need it:
It fixes the UX that would lead a user to think that there was some error in executing the
runcommandWhich issue(s) this PR fixes:
Fixes #5881
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer:
odo devodo devwithCtrl+C; odo should not exhibit the erroneous UX