Conversation
- adjust MachinePoolPredicates to use Template from jclouds
rdowner
left a comment
rdowner
left a comment
There was a problem hiding this comment.
Hi @andreaturli - thanks for this. In addition to the specific comments, I have a couple of general ones.
Firstly it looks like some of the tests still needs updates to work with the new jclouds stub provider. Picking one test failure at random, I saw expected [2.1.1.2] but found [144.175.1.21].
Secondly there's quite a few whitespace changes mixed in with "real" changes. It would be helpful if whitespace changes could be moved into a separate commit to ease reviewing. (But this is not a blocker to merging this PR.)
Thanks
Richard.
| !template.getOsFamily().equals(m.getOperatingSystem().getFamily())) return false; | ||
| !template.getImage().getOperatingSystem().getFamily().equals(m.getOperatingSystem().getFamily())) return false; | ||
| } | ||
| if (template.getOsNameMatchesRegex()!=null) { |
There was a problem hiding this comment.
This deleted line does not have an equivalent in the new version. Does this mean a potential a change in behaviour?
| HostAndPort hostAndPort, Iterable<LoginCredentials> credentialsToTry, ConfigBag setup) { | ||
| String waitForSshable = setup.get(WAIT_FOR_SSHABLE); | ||
| checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForSshable called despite waitForSshable=%s for %s", waitForSshable, hostAndPort); | ||
| // checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForSshable called despite waitForSshable=%s for %s", waitForSshable, hostAndPort); |
There was a problem hiding this comment.
Please avoid committing commented-out lines of code. Is this line supposed to be deleted, or was it commented out to help debugging?
| } | ||
|
|
||
| @Test | ||
| @Test(enabled = false) |
There was a problem hiding this comment.
Is there a reason for disabling this test? If so, please add a comment.
andreaturli
changed the title
|
retest this please |
1 similar comment
|
retest this please |
|
@andreaturli @richardcloudsoft Can you take a look at this please to see if it is still relevant, and address the comments above if appropriate Thanks |
4 participants
based on #807