Created a spinner for let users wait that the test is running #148
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @Bharadwajshivam28. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
k8s-ci-robot
added
needs-ok-to-test
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
| @@ -41,7 +48,7 @@ func PrintInfo(clientSet *kubernetes.Clientset, config *rest.Config) { | |||
| viper.Set("busybox-image", busyboxImage) | |||
| } | |||
|
|
|||
| log.Printf("API endpoint : %s", config.Host) | |||
| log.PrintfAPI("API endpoint : %s", config.Host) | |||
There was a problem hiding this comment.
By moving the spinner we can revert this to a normal printf and remove the PrintAPI method.
There was a problem hiding this comment.
Hey @rjsadow I made a new method PrintAPI because I wanted the Spinner to be at top most and below this the function to be running and when using Printf the output is-
and when i create separate method for API line the output is-
My end goal is to keep the spinner at top most and everything below it and if i use the common Printf and add a new line in that then all the lines will have a new line..
Please correct me if i am wrong
pkg/common/args.go
Outdated
| spinner := NewSpinner(os.Stdout) | ||
| spinner.Start() |
There was a problem hiding this comment.
Instead of having the spinner start here, I'd like to see it moved just around where we get the pod logs, that's by far the most significant stretch of time a user would be looking for input on if the command is running or has stopped/frozen/exited.
Instead of running NewSpinner and Start here in PrintInfo, lets move it to root.go and wrap the PrintE2ELogs function, like:
spinner := common.NewSpinner(os.Stdout)
spinner.Start()
client.PrintE2ELogs()
spinner.Stop()
There was a problem hiding this comment.
can you please expand this? sorry i not got it..
I created a separate file for spinner in common directory so are you saying me to move the start method in the root.go file? and use the method created in root.go in the file args.go file?
pkg/common/args.go
Outdated
| @@ -87,3 +94,102 @@ func ValidateArgs() error { | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| var spinnerFrames = []string{ | |||
There was a problem hiding this comment.
Lets move all this code for the spinner to a new spinner.go file, I'm alright with leaving it in the common package for now. Eventually we might want to move it to a utils or cli package.
There was a problem hiding this comment.
okay let me make the changes..
pkg/common/args.go
Outdated
| func (s *Spinner) SetPrefix(prefix string) { | ||
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
| s.prefix = prefix | ||
| } | ||
|
|
||
| func (s *Spinner) SetSuffix(suffix string) { | ||
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
| s.suffix = suffix | ||
| } |
There was a problem hiding this comment.
Are these methods used? If not, let's remove them.
There was a problem hiding this comment.
oh not saw these.. let me fix it
pkg/common/args.go
Outdated
| func (s *Spinner) Write(p []byte) (n int, err error) { | ||
| s.mu.Lock() | ||
| defer s.mu.Unlock() | ||
| if !s.running { | ||
| return s.writer.Write(p) | ||
| } | ||
| if _, err := s.writer.Write([]byte("\r")); err != nil { | ||
| return 0, err | ||
| } | ||
| return s.writer.Write(p) | ||
| } |
There was a problem hiding this comment.
Is this method needed? If not, lets remove it.
|
Hey @rjsadow I made the changes suggested by you.. |
Signed-off-by: shivam <[email protected]>
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Bharadwajshivam28, rjsadow 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 |
k8s-ci-robot
added
the
approved
There was a spinner missing which will indicate user that test is going on so they should wait.
Screencast.from.2024-02-11.06-39-28.webm
fixes - #118