You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Zack Shoylev <no...@github.com> on 2015/03/27 22:35:58 UTC

[jclouds-labs-openstack] Adds service predicates and more tests (#186)

You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds-labs-openstack/pull/186

-- Commit Summary --

  * Adds service predicates and more tests

-- File Changes --

    M openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/domain/ServiceStatus.java (10)
    A openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/predicates/ServicePredicates.java (61)
    M openstack-poppy/src/test/java/org/jclouds/openstack/poppy/v1/features/ServiceApiLiveTest.java (18)

-- Patch Links --

https://github.com/jclouds/jclouds-labs-openstack/pull/186.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/186.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/186

Re: [jclouds-labs-openstack] Adds service predicates and more tests (#186)

Posted by Zack Shoylev <no...@github.com>.
> +import static com.google.common.base.Preconditions.checkNotNull;
> +import static java.util.concurrent.TimeUnit.SECONDS;
> +import static org.jclouds.util.Predicates2.retry;
> +
> +import org.jclouds.openstack.poppy.v1.domain.Service;
> +import org.jclouds.openstack.poppy.v1.domain.ServiceStatus;
> +import org.jclouds.openstack.poppy.v1.features.ServiceApi;
> +import com.google.common.base.Predicate;
> +
> +public class ServicePredicates {
> +   public static Predicate<Service> awaitDeployed(ServiceApi serviceApi) {
> +      StatusUpdatedPredicate statusPredicate = new StatusUpdatedPredicate(serviceApi, ServiceStatus.DEPLOYED);
> +      return retry(statusPredicate, 1200, 15, 15, SECONDS);
> +   }
> +
> +   private static class StatusUpdatedPredicate implements Predicate<Service> {

I am not following here. I think it already is modeled after ServerStatusPredicate, and is already generalized, based on:
https://github.com/rackerlabs/jclouds-labs-openstack/blob/service-predicates/openstack-poppy/src/main/java/org/jclouds/openstack/poppy/v1/predicates/ServicePredicates.java#L30

So it will be easy to add another Predicate and just change the status from ServiceStatus.DEPLOYED to whatever is needed, as it is done in awaitDeployed.

Specifically, StatusUpdatedPredicate will accept any ServiceStatus to monitor.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/186/files#r27752603

Re: [jclouds-labs-openstack] Adds service predicates and more tests (#186)

Posted by Everett Toews <no...@github.com>.
:+1: after a check for naming consistency is done.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/186#issuecomment-89410171

Re: [jclouds-labs-openstack] Adds service predicates and more tests (#186)

Posted by Zack Shoylev <no...@github.com>.
Some rebase problems

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/186#issuecomment-89426143

Re: [jclouds-labs-openstack] Adds service predicates and more tests (#186)

Posted by Everett Toews <no...@github.com>.
> +import static com.google.common.base.Preconditions.checkNotNull;
> +import static java.util.concurrent.TimeUnit.SECONDS;
> +import static org.jclouds.util.Predicates2.retry;
> +
> +import org.jclouds.openstack.poppy.v1.domain.Service;
> +import org.jclouds.openstack.poppy.v1.domain.ServiceStatus;
> +import org.jclouds.openstack.poppy.v1.features.ServiceApi;
> +import com.google.common.base.Predicate;
> +
> +public class ServicePredicates {
> +   public static Predicate<Service> awaitDeployed(ServiceApi serviceApi) {
> +      StatusUpdatedPredicate statusPredicate = new StatusUpdatedPredicate(serviceApi, ServiceStatus.DEPLOYED);
> +      return retry(statusPredicate, 1200, 15, 15, SECONDS);
> +   }
> +
> +   private static class StatusUpdatedPredicate implements Predicate<Service> {

This should really be a more generalized status predicate like [ServerStatusPredicate](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/predicates/ServerPredicates.java#L92). The convenience method `awaitDeployed` can then be built on that.

I know it's unlikely that anyone would ever want to await on any of the other `ServiceStatus` in this particular case but the general style should still be followed.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/186/files#r27735322

Re: [jclouds-labs-openstack] Adds service predicates and more tests (#186)

Posted by Zack Shoylev <no...@github.com>.
merged and backported

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/186#issuecomment-89426613

Re: [jclouds-labs-openstack] Adds service predicates and more tests (#186)

Posted by Everett Toews <no...@github.com>.
> +import static com.google.common.base.Preconditions.checkNotNull;
> +import static java.util.concurrent.TimeUnit.SECONDS;
> +import static org.jclouds.util.Predicates2.retry;
> +
> +import org.jclouds.openstack.poppy.v1.domain.Service;
> +import org.jclouds.openstack.poppy.v1.domain.ServiceStatus;
> +import org.jclouds.openstack.poppy.v1.features.ServiceApi;
> +import com.google.common.base.Predicate;
> +
> +public class ServicePredicates {
> +   public static Predicate<Service> awaitDeployed(ServiceApi serviceApi) {
> +      StatusUpdatedPredicate statusPredicate = new StatusUpdatedPredicate(serviceApi, ServiceStatus.DEPLOYED);
> +      return retry(statusPredicate, 1200, 15, 15, SECONDS);
> +   }
> +
> +   private static class StatusUpdatedPredicate implements Predicate<Service> {

Oops. I see that now. I think it was the name of the class that threw me. To me `StatusUpdatedPredicate` says it has to do with a 'Updated' status. Search the other predicates packages and see if this name is consistent with others. It should be inline with whatever is most consistent.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/186/files#r27753065