jclouds-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ignasi Barrera <notificati...@github.com>
Subject Re: [jclouds/jclouds] [JCLOUDS-1318] fix based on nodeTerminatePredicate (#1117)
Date Mon, 17 Jul 2017 06:12:35 GMT
nacx requested changes on this pull request.

I've just looked at the last changes. I'll run a local build too and see what happens with
that slow test.

>           }
+      } else if (templateOptions.getKeyPairName() != null && templateOptions.getLoginPrivateKey()
!= null) {

Why checking also the private key here? If users configure a key pair we don't care if they
also configure the private key.  Creating a node and not accessing it is a valid use case.
Perhaps users access it in another connection (where the private key will be configured).
I'd remove the private key check from here and always set the keypair when its name is set.

>  
-      boolean keyPairExtensionPresent = novaApi.getKeyPairApi(region).isPresent();
+      List<Integer> inboundPorts = Ints.asList(templateOptions.getInboundPorts());
+      if (!templateOptions.getGroups().isEmpty() && !inboundPorts.isEmpty()) {

This should be `||`.

-- 
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/pull/1117#pullrequestreview-50241023
Mime
View raw message