kubernetes-client icon indicating copy to clipboard operation
kubernetes-client copied to clipboard

Watcher interface could be a Functional interface

Open lordofthejars opened this issue 4 years ago • 7 comments

I'd like to be able to generate a Watcher in my kubernetes operator as a functional interface/Method reference instead of having to use an inner class.

crClient.watch(new Watcher<PizzaResource>() {
            @Override
            public void eventReceived(Action action, PizzaResource resource) {

Now it is not a functional interface because it has two methods to implement:

public interface Watcher<T> {

  void eventReceived(Action action, T resource);

  void onClose(KubernetesClientException cause);
}

So I purpose to make the onClose method as default method where the implementation is either empty or

default void onClose(KubernetesClientException cause) {
  if (cause != null) {
     cause.printStackTrace();
    System.exit(-1);
  }
}

At the end the developer is free to override this method, but if it is fine for him, he will be able to use functional interface capabilities introduced in Java8.

If you agree I can provide a PR.

lordofthejars avatar Jun 09 '20 10:06 lordofthejars

@iocanel @oscerd @manusa : What do you think about this?

rohanKanojia avatar Jun 09 '20 11:06 rohanKanojia

Something like this looks kind of neat:

watch((action, t ) -> doSomething(t));

But I'm worried we're encouraging users to ignore the onClose event altogether. Maybe we should add something in the JavaDoc too warning about the possible problems of ignoring the onClose implementation. Also not sure about the System.exit(-1); default.

manusa avatar Jun 09 '20 14:06 manusa

This is what I used in all the operators I developed but of course we can try to think in a better default method.

lordofthejars avatar Jun 09 '20 14:06 lordofthejars

In regard to System.exit(-1); I just find it a very aggressive approach (taking into account it's the default), and I'm not sure if other users may encounter problems due to this. For an operator it will be OK (I guess) because the Pod will eventually restart and there'll probably be more than one instance and will usually be designed for failure. I'm not sure if there are other use-cases (probably less cloud friendly) where this can cause some problems.

Anyways, given I tend to abuse the java Functional API, I'm all for it.

manusa avatar Jun 09 '20 14:06 manusa

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

stale[bot] avatar Sep 07 '20 15:09 stale[bot]

Well System.exit is aggressive I know but at the end it is a Pod, I think it is a good approach as an exception means something went wrong (really wrong) so I think that a restart is the best default approach. After all, anyone is free to override this behaviour.

lordofthejars avatar Sep 16 '20 10:09 lordofthejars

One domain where System.exit is annoying is during tests where this doesn't let the test framework correctly report the problem and clean things up.

metacosm avatar Nov 20 '20 10:11 metacosm

This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!

stale[bot] avatar Aug 16 '22 00:08 stale[bot]