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 [email protected] or file a JIRA ticket
with INFRA.
---
