Fix event recorder not to log events without a reference #5248
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check the information on your CLA or see this help article on setting the email on your git commits. Once you've done that, please reply here to let us know. If you signed the CLA as a corporation, please let us know the company's name. |
|
@lavalamp PTAL |
| @@ -197,6 +197,7 @@ func (i *recorderImpl) Event(object runtime.Object, reason, message string) { | |||
| ref, err := api.GetReference(object) | |||
| if err != nil { | |||
| glog.Errorf("Could not construct reference to: '%#v' due to: '%v'. Will not report event: '%v' '%v'", object, err, reason, message) | |||
| return | |||
There was a problem hiding this comment.
Should Event just return an error instead?
There was a problem hiding this comment.
I guess this is good to bring back to old behavior.
There was a problem hiding this comment.
Yeah, callers of Event/Eventf aren't really equipped to do something better with the error.
|
LGTM - merging this because it just restores behavior from a prior regression. |
Fix event recorder not to log events without a reference
#4974 removed a return statement that prevented panics when an ObjectReference couldn't be obtained from an object passed to
Event.