Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't wrap existing client with WithWatch() #2967

Closed
2uasimojo opened this issue Sep 27, 2024 · 3 comments
Closed

Can't wrap existing client with WithWatch() #2967

2uasimojo opened this issue Sep 27, 2024 · 3 comments

Comments

@2uasimojo
Copy link
Contributor

In some cases, one can wrap already-extant client.Clients to confer extra functionality. For example:

cWithFieldOwner := client.WithFieldOwner(cWithoutFieldOwner, "owner")

However, it is not possible to confer WithWatch on an existing client: one can only obtain a client.WithWatch via a constructor:

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()!

It would be great to have a decorator function to allow an existing client to grow WithWatch functionality:

c, err := client.New(cfg, opts)
c = client.WithFieldOwner(c, "owner")
// ... chain other decorators ...
cWithAllTheThings, err := client.AsWithWatch(c)

Notes:

  • It would be better to call the wrapper WithWatch, but that name is already taken by the interface. Sad face.
  • I can't think of a way to get around this client being "the end of the line". It can't be further wrapped, unless the wrapper is WithWatch-specific. Sad face.
  • It would be great if you could do this to any client.Client; but as written today that would be hard because the Watch() implementation relies on internals of the specific client (lowercase) implementation in the library. Sad face.

See #2929 where I prototyped an imperfect version of the solution that suffers from all of the above issues.

@alvaroaleman
Copy link
Member

Can we use generics for this?

@alvaroaleman
Copy link
Member

To expand on what I was thinking, I thought we might be able to have some sort of wrapper type for the various options that takes a client.Client or client.WithWatch through generics and returns whatever it has gotten. The main drawback is that the return will always be assertable to WithWatch even if it it isn't.

Note that this plays into #2888, we should probably solve the two problems together

@2uasimojo
Copy link
Contributor Author

Cool, yeah, it makes sense to sweep this up with 2888 -- I'll close this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants