-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Update glogging to always set level, and to respect glog.level #8924
Update glogging to always set level, and to respect glog.level #8924
Conversation
23e7eb8
to
52b73a4
Compare
[testonlyextended][extended:core(should allow to change build log level)] |
[test] |
@bparees @gabemontero fixes the failing extended test |
@smarterclayton @bparees I'll check the origin_extended_build_test job once this merges |
Need a sign off |
} | ||
glog.Infof("Pushing %s image ...", pushTag) | ||
glog.V(0).Infof("Pushing %s image ...", pushTag) |
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.
V(1) ?
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.
Printed at 0 because it's the slowest part of the operation, so always shown.
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 1 already always shown? I was under the impression we were using 0 for warning/erring, and 1 for basic info (still always shown by default).
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.
52b73a4
to
4b2104d
Compare
@@ -116,11 +116,11 @@ func (d *DockerBuilder) Build() error { | |||
if authPresent { | |||
glog.V(4).Infof("Authenticating Docker push with user %q", pushAuthConfig.Username) | |||
} | |||
glog.Infof("Pushing image %s ...", pushTag) | |||
glog.V(1).Infof("Pushing image %s ...", pushTag) |
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.
well you've got this push as a v(1) and the other push as a v(0)
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.
Ok
[test] On Mon, May 23, 2016 at 12:00 PM, OpenShift Bot [email protected]
|
4b2104d
to
1b10493
Compare
Evaluated for origin testonlyextended up to 1b10493 |
continuous-integration/openshift-jenkins/testonlyextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin_extended/189/) (Extended Tests: core(should allow to change build log level)) |
Wtf is this:
On Mon, May 23, 2016 at 3:19 PM, OpenShift Bot [email protected]
|
@Kargakis this does not belong here. |
I'd like this patched out. |
[test] |
Evaluated for origin test up to 1b10493 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3993/) |
I think we can have it in the factory so we won't need to carry a patch. |
nope, lgtm. |
[merge] |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4099/) (Image: devenv-rhel7_4280) |
Evaluated for origin merge up to 1b10493 |
Fixes #8903 partially