-
Notifications
You must be signed in to change notification settings - Fork 20
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
Print a message if a stage is taking more than 10 seconds #185
Conversation
Signed-off-by: Itxaka <[email protected]>
Signed-off-by: Itxaka <[email protected]>
Signed-off-by: Itxaka <[email protected]>
|
so the problem seems that in the case of the rsync we needed the channel to pass back the info about the rsync output, but here we just need to print a info message, nothing more |
pkg/console/console.go
Outdated
out, err := c.CombinedOutput() | ||
|
||
// Stop the timer log | ||
cancel() |
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.
isn't this redundant? we do already call defer cancel()
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.
umm that its true, maybe I was testing this manually or something?
right, but the new changes looks good - with the first round we were keeping the goroutine alive because it was not being closed at all. Now we bind it to the context which gets canceled after we finish running the command (which makes sense). |
Just to call it out explicitly here: this covers only shell commands, not all other plugins. Can't we make at this point hook at the plugin execution? that way could cover also other cases (e.g. fetching userdata over slow network, etc) |
I dont know tbh, its it possible for other sources to take this long? We are talking seconds here for a "big" delay, so maybe other sources are not prone to be this slow? While commands can be really slow, like a script that waits for a service to be up and such? But yeah, it may be trivial to adapt this to it so maybe its possible to just plugin it in before the plugin is executed |
Signed-off-by: Itxaka <[email protected]>
1849eee
to
2cb0329
Compare
@mudler how about now?
on info:
|
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.
Perfect! thank you @Itxaka !
With a config like this:
This is the output: