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

Implement waitUntilCondition #989

Closed
iocanel opened this issue Jan 15, 2018 · 8 comments
Closed

Implement waitUntilCondition #989

iocanel opened this issue Jan 15, 2018 · 8 comments
Assignees
Labels
Projects
Milestone

Comments

@iocanel
Copy link
Member

iocanel commented Jan 15, 2018

Currently we have the waitUntilReady() functionality, which uses a hybrid approach with polling periodically and watches. This works great but it doesn't cover all our needs. Fro example what about waiting for builds or jobs or anything.

I'd like us to introduce a new method on all resources called waitUntil that will accept and argument, possibly something like a Function<KubernetesResource, Boolean> which is going to return true when the condition is meet.

Then waitUntilReady should be build on top of that.

@hrishin hrishin added this to the Sprint 144 H milestone Jan 24, 2018
@piyush-garg
Copy link
Contributor

piyush-garg commented Feb 1, 2018

Hi @iocanel,

Right now we have this functionality in client,

waitUntilReady() {
   waitUntilReady(amount, Timeout);
}

waitUntilReady(amount, TimeUnit) {
   waitTill(timeout or resource get added to queue);
}

So the requirement is something like

waitUntilReady(condition) {
   waitUntillCondition(condition);
}

waitUntilCondition(condition) {
   wait till the condition return true
}

Or

waitUntilReady(condition) {
   wait till waitUntillCondition(conditon) return true;
}

waitUntilCondition(condition) {
   return whether the condition is true or false;
}

Like is the way I am thinking is fine or you have something else as a prototype.

I am assuming that user will provide the condition. This may be wrong also.

Please guide the way this needs to be done.

Thanks

cc @hrishin @rohanKanojia @oscerd

@iocanel
Copy link
Member Author

iocanel commented Feb 2, 2018

I would put it slightly different:

At the moment we have:

public T waitUntilReady(long amount, TimeUnit timeUnit) {
    //do wait until ready and then return resource.
}

There are people asking to add things like:

public T waitUntilComplete(long amount, TimeUnit timeUnit) {
    //pretty much do the same thing as waitUntilReady but this time until the state is complete.
}

and

public T waitUntilStarted(long amount, TimeUnit timeUnit) {
    //pretty much do the same thing as waitUntilReady but this time until the job has started
}

So, this issue actually proposes that instead of copying the same mechanism to 3 or more similar methods, to expose it as a generic method:

public void T waitUntilCondition(Function<T, Boolean> condition, long amount, TimeUnit timeunit) {
    //poll & watch until condition applied to the resource returns true.
}

The all the rest waitForSomething methods can internally reuse waitUntilCondition.

For instance watiUntilReady can become:

 public void T waitUntilReady(long amount, TimeUnit timeUnit) {
     waitUntilCondition(r -> Readiniess.isReady(r), amount, timeUnit);
 }

@piyush-garg piyush-garg added this to In progress in Project FKC Feb 26, 2018
@piyush-garg piyush-garg added this to Sprint-146 Progress in Build Team Mar 15, 2018
@piyush-garg piyush-garg removed this from Sprint-146 Progress in Build Team Mar 28, 2018
@piyush-garg
Copy link
Contributor

@iocanel Thanks for your input.

As explained above, lambda expressions can be used in Java 8 but our client is in Java 7. Either we need to move to Java 8 or we can use Functional Interface, in that case, the user needs to implement the function, that does not seem to be a good way to do that.

@iocanel Can you provide more thoughts on this or Am I proceeding in some wrong direction?

Thanks

@iocanel
Copy link
Member Author

iocanel commented Mar 28, 2018

if we want to stick with java7 we can use the Function class that comes with the kubernetes-model (inside the builder package). Of course, in this case there will be no lambda support.

I'd rather see us moving to java8 though.

@sthaha sthaha added this to Backlog in Build Team Apr 3, 2018
@piyush-garg piyush-garg removed the wip label May 29, 2018
@iocanel iocanel self-assigned this Oct 9, 2018
@geoand
Copy link
Contributor

geoand commented Oct 9, 2018

I will start working on this

@rhuss
Copy link
Contributor

rhuss commented Oct 10, 2018

@iocanel @rohanKanojia wdyt about updating the major version and move to Java 8 ? A bit similar as we do for fmp where we also moved to Java 8 ?

For sure we should skip Java 7 support asap, as they world finally fully embraced Java 8 and is moving quickly to Java 11 as it seems.

@geoand
Copy link
Contributor

geoand commented Oct 10, 2018

@rhuss Hasn't the project already moved to Java 8? I was under the impression that #1168 addressed that concern

@rhuss
Copy link
Contributor

rhuss commented Oct 10, 2018

Ah, sorry my bad. haven't checked the date and project state and tried to answer #989 (comment)

I need more coffee ....

@rohanKanojia rohanKanojia moved this from In progress to Done in Project FKC Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Project FKC
  
Done
Development

No branches or pull requests

6 participants