Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/782#discussion_r35936345
--- Diff:
software/nosql/src/main/java/brooklyn/entity/nosql/redis/RedisClusterImpl.java
---
@@ -23,9 +23,13 @@
import brooklyn.entity.Entity;
import brooklyn.entity.basic.AbstractEntity;
import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityInternal;
import brooklyn.entity.group.DynamicCluster;
import brooklyn.entity.proxying.EntitySpec;
import brooklyn.entity.trait.Startable;
+import brooklyn.event.AttributeSensor;
+import brooklyn.event.SensorEvent;
+import brooklyn.event.SensorEventListener;
import brooklyn.location.Location;
public class RedisClusterImpl extends AbstractEntity implements
RedisCluster {
--- End diff --
Also, looking at the existing `RedisClusterImpl` code, the use of fields
for `master` and `slaves` is going to cause problems on Brooklyn restart (or HA
failover). The fields won't get reset. Those need to be stored as sensors, or
the getter methods need to look up the values by searching the children.
Caching it in a field is fine, obviously.
Note that HA matters a lot to the customer who is driving this, so we need
to test that before we give it to him for his lab.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
