✨ Add client.WithWatch wrapper #2929
Conversation
In some cases, one can wrap already-extant `client.Client`s to confer extra functionality. For example: ```go cWithFieldOwner := client.WithFieldOwner(cWithoutFieldOwner, "owner") ``` However, it has not heretofore been possible to confer `WithWatch` on an existing client: one could only obtain a `client.WithWatch` via a constructor: ```go cWithWatch, err := client.NewWithWatch(cfg, opts) ``` The resulting client is "frozen" -- i.e. can't be subjected to decorators like `WithFieldOwner` -- because those wrappers return a `client.Client`, which can't `Watch()`! This commit adds a decorator function to allow an existing client to grow `WithWatch` functionality: ```go c, err := client.New(cfg, opts) c = client.WithFieldOwner(c, "owner") // ... chain other decorators ... cWithAllTheThings, err := client.AsWithWatch(c) ``` Notes: - We would have preferred to call the wrapper `WithWatch`, but that name is already taken by the interface. Sad face. - This client is "the end of the line". It can't be further wrapped, unless the wrapper is `WithWatch`-specific. Sad face. - As currently conceived, you can't just do this to any `client.Client` because the `Watch()` implementation relies on internals of the specific `client` (lowercase) implementation in the library. Sad face.
k8s-ci-robot
added
do-not-merge/work-in-progress
|
Hi @2uasimojo. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
the
needs-ok-to-test
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 2uasimojo The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/S
|
Reviewers: I don't think this PR is acceptable without addressing at least the second and third Notes in the description. By the way, here's the concrete use case that led to this: https://github.com/openshift/hive/pull/2416/files#diff-dd2b5dc061a42ad471ddce4733423593aa40a07fc2e877b76abccee42233a11cR14-R23 |
|
This prototype is pretty far from a good solution. I'm hoping whatever comes out of #2888 will be clean and pretty. |
In some cases, one can wrap already-extant
client.Clients to confer extra functionality. For example:However, it has not heretofore been possible to confer
WithWatchon an existing client: one could only obtain aclient.WithWatchvia a constructor:The resulting client is "frozen" -- i.e. can't be subjected to decorators like
WithFieldOwner-- because those wrappers return aclient.Client, which can'tWatch()!This commit adds a decorator function to allow an existing client to grow
WithWatchfunctionality:Notes:
WithWatch, but that name is already taken by the interface. Sad face.WithWatch-specific. Sad face.client.Clientbecause theWatch()implementation relies on internals of the specificclient(lowercase) implementation in the library. Sad face.Closes #2967