Mailing-List: contact dev-help@brooklyn.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@brooklyn.incubator.apache.org
From: ahgittin <git@git.apache.org>
To: dev@brooklyn.incubator.apache.org
Reply-To: dev@brooklyn.incubator.apache.org
References: <git-pr-108-incubator-brooklyn@git.apache.org>
In-Reply-To: <git-pr-108-incubator-brooklyn@git.apache.org>
Subject: [GitHub] incubator-brooklyn pull request: Persist feeds
Content-Type: text/plain
Message-Id: <20140805143558.0D9C39BF988@tyr.zones.apache.org>
Date: Tue,  5 Aug 2014 14:35:58 +0000 (UTC)

Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/108#issuecomment-51205959
  
    looks real good.  some comments:
    
    * can we make `Feed` an `EntityAdjunct` for consistency (and ultimately for better display in the GUI) ?
    * can you build on #106 so we get `uniqueTag` and tags and prevention of duplicate addition?
    * for things like `JmxFeed` do we need to do some special things to recreate the `JMXConnection` which I'd be surprised if it serializes (though presumably you've tested this) and to minimize the number of fields which need to be persisted?  (ie declare more things transient and ensure they are populated on first use)
    * is there any way to warn if a feed is not explicitly attached to an entity?
    * with the above we should have another pass at removing redundant cleanup logic (and connecting feeds in init, with "service_up" guards as you suggest)
    
    Re the last point this will be easier once some of the things I'm working on for intermediate `SERVICE_UP` sensors like `SERVICE_PROCESS_IS_RUNNING` is completed (soon!).
    
    You also asked whether switching to adjunct means that feeds should be persisted separately to the entities.  This feels like a good idea, e.g. in cases where feeds break.  (A feed seems more brittle to me than enrichers -- and both feel quite similar.)  Hopefully with `BrooklynObjectType` this becomes relatively easy.
    
    Longer term it might be nice to restructure persistence so that all adjuncts (except for shared locations?) are stored underneath the entity, e.g. directory structure:
    
        applications/
          12345678/
            entities/
              12345678/
                entity.xml
                enrichers/
                  ...
                feeds/
                  ...
    
    (but we have to be careful not to break existing persistence -- maybe support both, with adjunct dirs off the root always read, and written for items which aren't attached to entities, e.g. shared locations, but associated adjuncts moved when rewritten?)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---