-
Notifications
You must be signed in to change notification settings - Fork 314
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
supervisor and svc pkg versions in sup log output #5666
supervisor and svc pkg versions in sup log output #5666
Conversation
This change is aimed at humans reading Habitat Supervisor logs and wishing to know precise versions of the Supervisor and the Services that are running. Signed-off-by: Jeremy J. Miller <jm@chef.io>
Signed-off-by: Jeremy J. Miller <jm@chef.io>
It appears to me that the AppVeyor build failed with an unrelated error:
|
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.
@jeremymv2 This would be a great improvement to the logging output. I think we can simplify the code a bit here, but then this would be great to merge.
Thanks again!
components/sup/src/manager/mod.rs
Outdated
.release | ||
.as_ref() | ||
.unwrap_or(&"UNKNOWN".to_string()) | ||
); |
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.
I think you should be able to replace all this with
outputln!("pkg version {}", service.pkg.ident);
since PkgIdent
implements Display
.
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.
Thinking a bit more, In terms of unifying the log output more, it may be better to remove the outputln!
call from line 585 and do it all down here:
outputln!("Starting {} ({})", spec.ident, service.pkg.ident);
That would give us a single line like this:
hab-sup(MR): Starting core/redis (core/redis/3.2.4/20170514150022)
rather than
hab-sup(MR): Starting core/redis
hab-sup(MR): pkg version core/redis/3.2.4/20170514150022
@jeremymv2 Yeah, I think that error is unrelated to this work. |
Signed-off-by: Jeremy J. Miller <jm@chef.io>
I like your idea @christophermaier
|
Obvious fix; these changes are the result of automation not creative thinking.
This change is targeted to humans reading Habitat Supervisor
logs whom wish to know the precise version of the Supervisor
and the Services that are managed.
Signed-off-by: Jeremy J. Miller jm@chef.io