Aled Sage

Results 8 comments of Aled Sage

Thanks @mikezaccardo - looking good. A few minor comments, some of which should be addressed and many of which should be deferred. You said in the description that the live...

@alasdairhodge Looks good. Only one important comment about reusing the `ManagementContext`s ExecutionManager, rather than creating a new one.

Actually, can we have a test that externalised configuration now works in locations? As I recall, it was only in one situation that it failed - when the config was...

How about a test like this in `ExternalConfigYamlTest`: ``` @Test public void testExternalisedLocationConfigAccessedDuringLocationInit() throws Exception { String yaml = Joiner.on("\n").join( "services:", "- type: org.apache.brooklyn.core.test.entity.TestApplication", "location:", " aws-ec2:", " identity: myidentity",...

@alasdairhodge any progress on including that test and getting it to pass? Ping me if you want to discuss or pair.

https://github.com/apache/incubator-brooklyn/pull/1158 contains some of these same changes (it switches to using the pure-java winrm4j client v0.2.0).

@ahgittin with such a huge pull request, I think we should definitely stick with review before merging. We are hopefully going to find time next week to review this properly,...

LGTM; happy for this to be merged (once confusion with the identical-looking https://github.com/apache/brooklyn-server/pull/1006 is cleared up).