Add a simple informer implementation. #28
Conversation
k8s-ci-robot
added
approved
kubernetes/config/azure.go
Outdated
| ExpiresIn: json.Number(l.user.AuthProvider.Config["expires-in"]), | ||
| ExpiresOn: json.Number(l.user.AuthProvider.Config["expires-in"]), | ||
| ExpiresIn: l.user.AuthProvider.Config["expires-in"], | ||
| ExpiresOn: l.user.AuthProvider.Config["expires-in"], |
There was a problem hiding this comment.
should it be "expires-on"?
|
|
||
| func (c *Cache) Run() { | ||
| for { | ||
| if err := c.ListWatch(); err != nil { |
There was a problem hiding this comment.
exponential backoff? otherwise there could be hot looping.
kubernetes/client/cache.go
Outdated
| if res.Type == "ADDED" || res.Type == "MODIFIED" { | ||
| c.AddOrUpdate(metadata, res.Object) | ||
| return | ||
| } else if res.Type == "DELETED" { |
There was a problem hiding this comment.
no need for 'else' as there is 'return' in the 'if' clause above
kubernetes/client/cache.go
Outdated
| // Lister is an interface for things that can list objects for all namespaces or by namespace | ||
| type Lister interface { | ||
| List() []interface{} | ||
| ByNamespace() []interface{} |
There was a problem hiding this comment.
does it need a namespace param?
There was a problem hiding this comment.
yep (and added 'implements' to catch these kind of errors in the future.
kubernetes/client/cache.go
Outdated
| ix := FindObject(list, metadata) | ||
| if ix == -1 { | ||
| return append(list, objEntry{metadata: metadata, obj: obj}), nil | ||
| } else { |
There was a problem hiding this comment.
no need for 'else' as there is 'return' above
| } | ||
| } | ||
|
|
||
| func Delete(list []objEntry, metadata *V1ObjectMeta) ([]objEntry, bool) { |
There was a problem hiding this comment.
do you mean to expose the function?
There was a problem hiding this comment.
going to add tests, so yes.
| return append(list[:ix], list[ix+1:]...), true | ||
| } | ||
|
|
||
| func FindObject(list []objEntry, metadata *V1ObjectMeta) int { |
There was a problem hiding this comment.
do you mean to expose the function?
kubernetes/client/cache.go
Outdated
|
|
||
| func (c *Cache) ProcessResult(res *Result) { | ||
| metadata := c.Extractor(res.Object) | ||
| if res.Type == "ADDED" || res.Type == "MODIFIED" { |
There was a problem hiding this comment.
better to define these event type constants somewhere (maybe in watch.go?), then reference it
kubernetes/client/cache.go
Outdated
| c.eventHandlers = append(c.eventHandlers, handler) | ||
| } | ||
|
|
||
| func (c *Cache) Run() { |
|
Comments addressed, please re-check. Thanks! |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns, yliaog The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Thanks for the review! |
No description provided.