search

brooklyncentral / brooklyn

This project has moved and is now part of the ASF
https://github.com/apache/incubator-brooklyn
72 stars 27 forks source link

Policy and Enricher rebind #1413

Closed grkvlt closed 10 years ago

grkvlt commented 10 years ago

Some initial work to add rebind capabilities for Enrichers and enable the persistence of Enrichers and Policies

buildhive commented 10 years ago

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

grkvlt commented 10 years ago

Will need rework after #1400 #1405 #1406 are merged

grkvlt commented 10 years ago

What is wrong with @buildhive :koala:

Caused by: java.lang.ClassNotFoundException: org.apache.maven.cli.MavenLoggerManager at org.codehaus.plexus.classworlds.strategy.SelfFirstStrategy.loadClass(SelfFirstStrategy.java:50) at org.codehaus.plexus.classworlds.realm.ClassRealm.unsynchronizedLoadClass(ClassRealm.java:259) at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass(ClassRealm.java:235) at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass(ClassRealm.java:227) ... 18 more channel stopped ERROR: Failed to parse POMs

richardcloudsoft commented 10 years ago

@grkvlt See my recent posts on the internal dev list. CloudBees have been notified of the problem and it should be resolved now.

I'll now close-and-reopen this PR and BuildHive should pass this time.

richardcloudsoft commented 10 years ago

Reopening to kick off BuildHive

buildhive commented 10 years ago

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

aledsage commented 10 years ago

PLEASE DO NOT MERGE INTO MASTER.

This enabled policy persistence, I believe, which is not yet working properly so will break things in big ways. However, it's great that the code is now shared - thanks @grkvlt

I'm not sure how painful enricher persistence will be for us. We have a big mix of enrichers being written either in init() (in which case they need persisted) or in connectSensors() (in which case that method is called again on rebind()). Therefore getting enricher persistence to work may be tricky without a lot of refactoring!

grkvlt commented 10 years ago

@aledsage that's why it has the in progress label, but admittedly I'm not sure that everyone would see that as meaning 'do not merge'

aledsage commented 10 years ago

@grkvlt looks good - added a few comments. I hadn't noticed the "in progress" label - nice, but agree not to be relied upon for reviewers. I'm going to merge this code into a branch on https://github.com/brooklyncentral/brooklyn so we can collaborate on it.

aledsage commented 10 years ago

I've merged an exact copy of this PR into the branch https://github.com/brooklyncentral/brooklyn/tree/feature/policy-persistence

See https://github.com/brooklyncentral/brooklyn/pull/1416.

This PR should be closed, but the comments are still relevant.

grkvlt commented 10 years ago

@aledsage will review the comments and then move work to your new branch and close this

aledsage commented 10 years ago

@grkvlt I started last night incorporating these comments into https://github.com/brooklyncentral/brooklyn/tree/feature/policy-persistence, and will finish that this morning - will ping you when that's done.

Closing this PR now.