nacx requested changes on this pull request.
> +import com.google.common.base.Supplier;
+import com.google.common.collect.Collections2;
+import com.google.inject.Inject;
+
+public class ImageSupplier<B> implements Supplier<Collection<Image>>,
Function<String, Image> {
+ private final Function<Collection<B>, Collection<B>> outdatedBoxesFilter;
+ private final VagrantBoxApiFacade.Factory<B> cliFactory;
+ private final Function<B, Image> boxToImage;
+
+ @Inject
+ ImageSupplier(Function<Collection<B>, Collection<B>> outdatedBoxesFilter,
+ VagrantBoxApiFacade.Factory<B> cliFactory,
+ Function<B, Image> boxToImage) {
+ this.outdatedBoxesFilter = checkNotNull(outdatedBoxesFilter,
"outdatedBoxesFilter");
+ this.cliFactory = checkNotNull(cliFactory, "cliFactory");
+ this.boxToImage = checkNotNull(boxToImage, "boxToImage");
Remove the redundant null checks.
> + protected Logger logger = Logger.NULL;
+
+ private final File home;
+ private final MachineConfig.Factory machineConfigFactory;
+ private final Supplier<Collection<Image>> imageLister;
+ private final Supplier<? extends Map<String, Hardware>> hardwareSupplier;
+
+ @Inject
+ VagrantExistingMachines(@Named(VagrantConstants.JCLOUDS_VAGRANT_HOME)
String home,
+ MachineConfig.Factory machineConfigFactory,
+ Supplier<Collection<Image>> imageLister,
+ Supplier<? extends Map<String, Hardware>> hardwareSupplier) {
+ this.home = new File(checkNotNull(home, "home"));
+ this.machineConfigFactory = checkNotNull(machineConfigFactory,
"machineConfigFactory");
+ this.imageLister = checkNotNull(imageLister, "imageLister");
+ this.hardwareSupplier = checkNotNull(hardwareSupplier,
"hardwareSupplier");
Remove the redundant null checks.
> this.timeSupplier = timeSupplier;
+ this.nodes.putAll(getExistingNodes(existingMachines));
It would be better to make the `VagrantExistingMachines` a supplier, to lazily
load them. Otherwise all the load and parsing happens at injection time, in the
constructor, where we should avoid throwing IO exceptions and other exceptions
that could be raised while loading the existing machines.
--
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-19043408
