Skip to content
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

Add event creation library and implement in scheduler. #1789

Merged
merged 4 commits into from
Oct 15, 2014

Conversation

lavalamp
Copy link
Member

No description provided.

@@ -73,6 +73,14 @@ type EndpointsInterface interface {
WatchEndpoints(ctx api.Context, label, field labels.Selector, resourceVersion string) (watch.Interface, error)
}

// EventInterface has methods to work with Event resources
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to pass context to these like the other method. I know you take issue with use of context in client, and I think your position is valid. But consistency please while we sort out that separate debate.

@erictune
Copy link
Member

Note to self. Don't merge this until #1564 is merged and then after namespace changes added to client.go and perhaps elsewhere.

@erictune
Copy link
Member

Am I right that there is no timestamp in an Event other than the object creation timestamp? Probably there was some discussion about why this is a good thing, though I cannot fathom why. Could you point me at that discussion?

// StartReporting starts sending events to recorder. Call once while initializing
// your binary. Subsequent calls will be ignored. The return value can be ignored
// or used to stop reporting, if desired.
func StartReporting(recorder EventRecorder, sourceName string) watch.Interface {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(copied from comment in your fork)
Are 'report' and 'record' being used as synonyms? If so, pick one. If not, explain the distinction.

@erictune
Copy link
Member

do kubecfg man pages need to be updated?

@lavalamp
Copy link
Member Author

Agree that events need a local timestamp in addition to the one they'll get on arrival at master, I realized that at some point but haven't fixed yet. I'll file an issue for that.

@lavalamp
Copy link
Member Author

Hm, can't find man pages for kubecfg? Looks like that got left out?

@lavalamp
Copy link
Member Author

OK, I think the only outstanding issue is what to do with contexts & namespaces. I would like to commit this as-is, and then wire namespaces up in a separate PR after @derekwaynecarr's PR gets merged.

@erictune
Copy link
Member

Please add the context in another PR soon.

erictune added a commit that referenced this pull request Oct 15, 2014
Add event creation library and implement in scheduler.
@erictune erictune merged commit 6f577aa into kubernetes:master Oct 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants