brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ahgittin <...@git.apache.org>
Subject [GitHub] brooklyn-server issue #338: Support nested catalog item definitions - testCa...
Date Tue, 28 Feb 2017 16:19:30 GMT
Github user ahgittin commented on the issue:

    https://github.com/apache/brooklyn-server/pull/338
  
    Very cool.  Haven't reviewed in depth but looks on point at a high level.  A few specific
comments:
    
    * this will conflict with https://github.com/apache/brooklyn-server/pull/573 -- should
be easy to resolve, and this is a better solution to the underlying problem
    
    * I think we want _both_ `catalogItemId` with the semantics of #573 and the `stack` introduced
here for searching, on the spec (that's what we have on entity)
    
    * there may be persistence errors migrating from older version if we persist specs and
remove the `catalogItemId` here (?); the previous suggestion to keep both should resolve this
(perhaps with an additional check when initializing an entity that if `catalogItemId` is set
then `stack.get(0)` equals that id
    
    And one general comment -- there is an issue if e.g. `my-cluster` and `my-node` are set
in the same BOM, both at version `1` with the former referring to the latter omitting the
version.  If we then install a version `2` of that bom, then `my-cluster:1` creates a cluster
of `my-node:2` which is surprising.  It would be nice to solve that (and also be able to retrieve
the `bom` file and bundle list where an entity is defined, for subsequent editting).  It's
a separate problem to this, but the solutions may be interrelated.  Very happy to have this
merged (once someone else does a detailed review and we resolve #573 conflicts) without solving
the version and catalog-group / bom questions, expecting a bit more change afterwards, but
wanted to flag that.  cc @geomacy @neykov @aledsage 


---
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.
---

Mime
View raw message