Fix rebind

[aledsage]

@ahgittin and @nakomis - I've address almost all of the comments comments in #1185, except the biggies. Still to do are:

1. where should we default the persistence-dir (e.g. PWD or ~/.brooklyn/)
2. support for running multiple separate concurrent brooklyn instances
3. we should have better handling of problems, including opportunity to fix / ignore / abort
4. we should add a message on failed rebind that the files for the entity will be ignored and left in place, that the problem can be fixed by adding missing jars to the classpath or editing the xml (and that gui/api support for selectively repairing may be available in future -- perhaps point to issue for that?). that just gives a user comfort...
5. make it easier to see when there are problems persisting entity's change-delta.
6. In the brooklyn.demo.WebClusterDatabaseExampleApp, the policy on the web cluster is not re-created, however the policy on the nginx controller is re-created
7. Error when rebinding to jClouds SshMachineLocation (as discussed on Skype). I experienced this issue with both the AgriWise project and the global web fabric example

And unrelated unaddressed comments:

1. CLI improvements
2. Couldn't run the whirr-hadoop example due to jclouds version conflict (lookout for a separate PR from me on this one)

@ahgittin for the problems you hit with entities-only-on-the-catalog-classpath, ControlledDynamicWebAppCluster failed to serialize because of classpath issues. When persisting the EntityMemento it tried to look up the Class.forName to find which config keys were statically defined (because it wouldn't persist those). I've fixed that by injecting the transient Class into the Memento to avoid the lookup.

[buildhive]

Brooklyn Central » brooklyn #1676 FAILURE
Looks like there's a problem with this pull request
(what's this?)

[sjcorbett]

For #1 - it should be configurable via properties, defaulting to a subdirectory of ~/.brooklyn. $PWD would just lead to directories scattered everywhere.

[ahgittin]

Test failure:

java.lang.AssertionError: Could not find on classpath: brooklyn/entity/rebind/brooklyn-AppInCatalog.jar expected object to not be null
    at org.testng.Assert.fail(Assert.java:94)
    at org.testng.Assert.assertNotNull(Assert.java:404)
    at brooklyn.entity.rebind.RebindCatalogEntityTest.setUp(RebindCatalogEntityTest.java:60)

Looking at the code now.

[ahgittin]

already done this 3 lines up!

[ahgittin]

code looks really good for addressing these, and tests - one trivial comment plus fix the test failure, then merge!

[ahgittin]

+1 @sjcorbett location configurable based on brooklyn.properties defaulting to a stable place (now PWD)

but worth some thought to think through how we will store and make files configurable -- note that we've got a BrooklynConfigKeys.BROOKLYN_DATA_DIR, which is used in BrooklynWebServer to decide where to place jetty files -- but we don't have a nice distinction between management server data dirs (currently under ~/.brooklyn/, perhaps ~/.brooklyn/mgmt/... as default for generated?) and target machine data/ops dirs (currently /tmp/brooklyn-xxx/ but i would move out of /tmp as default as we are better off asking people to clean up than risking their operational data gets blown away by a cron job!)

see #1172

[nakomis]

Having been bitten before (on a production server), definitely +1 to moving
out of /tmp. If @ahgittin hadn't realized that it was a cron job that
deleted our database, we'd still be scratching our heads

On 6 February 2014 14:42, ahgittin [email protected] wrote:

+1 @sjcorbett https://github.com/sjcorbett location configurable based
on brooklyn.properties defaulting to a stable place (now PWD)

but worth some thought to think through how we will store and make files
configurable -- note that we've got a BrooklynConfigKeys.BROOKLYN_DATA_DIR,
which is used in BrooklynWebServer to decide where to place jetty files --
but we don't have a nice distinction between management server data dirs
(currently under ~/.brooklyn/, perhaps ~/.brooklyn/mgmt/... as default for
generated?) and target machine data/ops dirs (currently /tmp/brooklyn-xxx/
but i would move out of /tmp as default as we are better off asking people
to clean up than risking their operational data gets blown away by a cron
job!)

see #1172 #1172

Reply to this email directly or view it on GitHubhttps://github.com//pull/1194#issuecomment-34328654
.

Martin Harris
Lead Software Engineer
Cloudsoft Corporation Ltd
www.cloudsoftcorp.com
Mobile: +44 (0)7989 047-855

[aledsage]

Have incorporate review comments, and fixed failing test (forgot to include a file!) - waiting for buildhive to confirm and then will merge.

[buildhive]

Brooklyn Central » brooklyn #1680 SUCCESS
This pull request looks good
(what's this?)