✨ adding APIReader to the manager and injection. #327
Conversation
| @@ -61,6 +61,9 @@ type controllerManager struct { | |||
| // client is the client injected into Controllers (and EventHandlers, Sources and Predicates). | |||
| client client.Client | |||
|
|
|||
| // apiReader is the reader that will make requests to the api server and not the cache. | |||
| apiReader client.Reader | |||
There was a problem hiding this comment.
Is there a reason for not making it client.Client ? [implementation wise client.Client is already supported]. Though it does get tricky with the naming if we decide to change the type here apiClient ? maybe directClient ?
There was a problem hiding this comment.
My thought process was
-
When you need this field you really only need the reader. This still points people to the client for most use cases. I was hoping the fact it did not have any write mechanisms people would only use this when they needed it.
-
Naming is the other reason. I think the idea that we don't really expose "direct" vs "cache" and try to hide this decision as much as possible is a good thing. This is just a step in adding an escape hatch but not exposing that decision still.
Thoughts?
There was a problem hiding this comment.
those reasons @shawn-hurley make a lot of sense -- +1: from me to having it be a Reader.
There was a problem hiding this comment.
Sorry for delayed response. Haven't come across write use case, so going with the restricted use-case sounds good.
7fdbe94 to
531b7e0
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: shawn-hurley 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 |
fixed gopkg.toml lookup code in vendor update
Adding the APIReader onto the manager and allowing users to use dependency injection to add this to their types.