I've had a look at the adapter and looks good. The ProvisioningManager approach looks clean! A couple questions:
* When is the ProvisioningManager closed? I haven't seen a call to its close method and we need to make sure its executors are closed. I'd suggest to take a slightly different approach, to bind the ProvisioningManager to the jclouds context lifecycle, but that would require some (minor) changes to make it better managed by Guice: * Configure the ProvisioningManager in a *provider* method in the Compute module, so it can be added to the jclouds Closer. Have a look at the [ExecutorServiceModule](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/concurrent/config/ExecutorServiceModule.java#L135-L138) to see an example. This way it will be closed then the jclouds context is closed. * Given that the ProvisioningJob is always about the datacenter, I would specialise it, and configure it with Guice so it gets the predicate already injected. That would leave the adapter code cleaner. To do that, you'll have to use the Guice assisted inject so you can provide the operation to run. It could be something like: ```java public class ProvisioningJob implements Job { public interface Factory { ProvisioningJob create(String group, Supplier<Object> operation); } private final Predicate<String> waitDataCenterUntilReady; private final String group; private final Supplier<Object> operation; @Inject ProvisioningJob(@Named(POLL_PREDICATE_DATACENTER) Predicate<String> waitDataCenterUntilReady, @Assisted String group, @Assisted Supplier<Object> operation) { this.waitDataCenterUntilReady = waitDataCenterUntilReady; this.group = checkNotNull(group, "group cannot be null"); this.operation = checkNotNull(operation, "operation cannot be null"); } @Override public Object call() throws Exception { waitDataCenterUntilReady.apply(group); Object obj = operation.get(); waitDataCenterUntilReady.apply(group); return obj; } } ``` Then, you should configure it in the ComputeService module *configure* method as follows: ```java install(new FactoryModuleBuilder().build(ProvisioningJob.Factory.class)); ``` And finally, use it in the compute service adapter, by injecting the job factory: ```java // Inject the job factory private final ProvisioningJob.Factory jobFactory; (...) // Use it to schedule a new job provisioningManager.provision(jobFactory.create(dataCenterId, new Supplier<Object>() { @Override public Object get() { return api.serverApi().createServer(serverRequest) }) ); ``` It's just a suggestion to keep the job creation more self-contained and injectable, and to make the code a bit more concise. Great job btw! I'll finish the review later today. Meanwhile, feel free to squash the commits and apply any changes you might have. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/145#issuecomment-106287652
