pkg/sdk: add watch API and informer implementation #9
Conversation
pkg/sdk/types/types.go
Outdated
| // last known state. | ||
| type Event struct { | ||
| Object Object | ||
| ObjectExist bool |
There was a problem hiding this comment.
I am curious how does user handle object that's being updated? does the user want also know about old obj too? or that doesn't matter.
There was a problem hiding this comment.
also would the event be more descriptive if we have enum on what type of event it is?
e.g:
type EventType int
const (
Added EventType = iota
Modified
Deleted
Error
)
type Event struct {
EventType
obj interface
}
There was a problem hiding this comment.
does the user want also know about old obj too? or that doesn't matter.
Doesn't matter.
also would the event be more descriptive if we have enum on what type of event it is?
That's out of the scope.
For design discussion, please comment on the design doc.
pkg/sdk/api.go
Outdated
| handler *stub.Handler | ||
| } | ||
|
|
||
| func NewSDK() SDK { |
There was a problem hiding this comment.
Just New. It will look like SDK.New( ) outside this package.
There was a problem hiding this comment.
Probably don't define a struct now?
User will call sdk.Watch() and sdk is the package name, not a struct.
There was a problem hiding this comment.
@hongchaodeng Yeah I'm a little confused on the implementation of that. There should be some instance of the SDK that is configured by Watch() and Handle() before you call sdk.Run(). That's what the sdk struct was for.
Or is this configuration for the entire pkg and not an instance of the sdk?
pkg/sdk/doc.go
Outdated
| @@ -0,0 +1,12 @@ | |||
| package sdk | |||
There was a problem hiding this comment.
Remove this doc?
We will have the example in README.
There was a problem hiding this comment.
I think it is the golang idiomatic way to include doc.go for each pkg to describe what the pkg does. e.g https://github.com/coreos/etcd/blob/master/clientv3/doc.go
| "k8s.io/apimachinery/pkg/runtime" | ||
| ) | ||
|
|
||
| type Object runtime.Object |
pkg/stub/handle.go
Outdated
| @@ -0,0 +1,20 @@ | |||
| package stub | |||
There was a problem hiding this comment.
Remove this?
The stub is generated.
pkg/sdk/types/types.go
Outdated
| // last known state. | ||
| type Event struct { | ||
| Object Object | ||
| ObjectExist bool |
There was a problem hiding this comment.
does the user want also know about old obj too? or that doesn't matter.
Doesn't matter.
also would the event be more descriptive if we have enum on what type of event it is?
That's out of the scope.
For design discussion, please comment on the design doc.
|
@hongchaodeng the sdk API is now at the package level. PTAL. |
pkg/sdk/api.go
Outdated
| for _, informer := range informers { | ||
| err := informer.Run(ctx, handler) | ||
| if err != nil { | ||
| panic("TODO") |
There was a problem hiding this comment.
Write a bit more TODO what?
pkg/sdk/types/types.go
Outdated
| ObjectExist bool | ||
| } | ||
|
|
||
| // FuncType defines the type of the function of an Action. |
There was a problem hiding this comment.
Can you leave these non-watch related changes out of this PR?
|
Removed non Watch related types like |
|
LGTM. |
4 participants
Follow up to #8 with the simplified API with Watch.
Again the focus of this PR is mainly how the Watch API is implemented through informers.
According to the 0.1 milestone design there is no dispatcher, and instead every informer invokes the Handle() function in its sync for each Event to get the required Actions.
A lot of things are still TODO, like initializing or creating the SDK, handling the actions, supporting label selector options for Watch etc.
Plus based on some offline discussion I've added a wrapper around the regular
context.Contextin anticipation of passing additional info to the Handler but it's usage is still unclear this early on.