Initial model adapter controller implementation #32
Conversation
Jeffwan
changed the title
Jeffwan
force-pushed
the
jiaxin/model-adapter-controller-implementation
branch
from
ca2c1ea to
5697c2e
Compare
| @@ -41,27 +70,64 @@ func Add(mgr manager.Manager) error { | |||
|
|
|||
| // newReconciler returns a new reconcile.Reconciler | |||
| func newReconciler(mgr manager.Manager) (reconcile.Reconciler, error) { | |||
| cacher := mgr.GetCache() | |||
|
|
|||
| podInformer, err := cacher.GetInformer(context.TODO(), &corev1.Pod{}) | |||
There was a problem hiding this comment.
We can do this but it will be difficult to write unit tests later. I am ok, with adding a TODO for later or you can add them function arguments.
There was a problem hiding this comment.
There is no business logic in this fuction, so we dont need to write unit test for it. Ignore the previous comment.
There was a problem hiding this comment.
I considered to use informer here directly but currently use the high level interface. may do some refactor later here. so this is indeed not being used yet.
| } | ||
|
|
||
| err := r.Get(ctx, objectKey, svc) | ||
| if err == nil { |
There was a problem hiding this comment.
you can remove the err == nil check.
Just one is if errors.IsNotFound() {} is enough.
There was a problem hiding this comment.
make sense. Let me update the logic here.
Jeffwan
changed the title
|
@varungup90 feedback addressed. |
* Add initial implementation of model adapter controller * Update adapter pod lists * Finish initial integration tests * Address review feedback
Address #31 and part of #22
DO NOT MERGE it until #27 is merged and I will rebase the code and update this PR