nacx commented on this pull request.
Looks good in general! I've added a few comments to complete my understanding
on how the provider works..
Could you also elaborate a bit the motivation behind having split the Vagrant
Facade in two different interfaces?
> this.timeSupplier = timeSupplier;
+ this.nodes = Suppliers.memoize(new
ConcurrentWrapperSupplier(existingMachines));
Would it make sense to configure this supplier (qualified with `@Memoized` if
appropriate) in a Guice module? It looks more flexible and correct to get
injected what you're gonna use instead of decorating the arguments?
> @@ -73,36 +93,41 @@ public long getDelay(TimeUnit unit) {
return unit.convert(expiryTime - timeSupplier.get(),
TimeUnit.MILLISECONDS);
}
}
- private Map<String, VagrantNode> nodes = new ConcurrentHashMap<String,
VagrantNode>();
- private DelayQueue<TerminatedNode> terminatedNodes = new
DelayQueue<TerminatedNode>();
+
+ private final DelayQueue<TerminatedNode> terminatedNodes = new
DelayQueue<TerminatedNode>();
Didn't know about this type of queue. The way it is used here looks awesome! :)
> @@ -73,36 +93,41 @@ public long getDelay(TimeUnit unit) {
return unit.convert(expiryTime - timeSupplier.get(),
TimeUnit.MILLISECONDS);
}
}
- private Map<String, VagrantNode> nodes = new ConcurrentHashMap<String,
VagrantNode>();
- private DelayQueue<TerminatedNode> terminatedNodes = new
DelayQueue<TerminatedNode>();
+
+ private final DelayQueue<TerminatedNode> terminatedNodes = new
DelayQueue<TerminatedNode>();
Just to understand the whole picture. Is this used as an in-memory cache of
recently terminated nodes (and why)? If not, is there any other purpose for
this?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/355#pullrequestreview-19429117
