brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aledsage <...@git.apache.org>
Subject [GitHub] brooklyn-server pull request #879: Elect primary / failover policies
Date Tue, 07 Nov 2017 12:35:04 GMT
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/879#discussion_r149312203
  
    --- Diff: api/src/main/java/org/apache/brooklyn/api/mgmt/ManagementContext.java ---
    @@ -309,8 +310,14 @@
         /** As {@link #lookup(String, Class)} but not constraining the return type */
         public BrooklynObject lookup(String id);
         
    -    /** Finds an entity with the given ID known at this management context */
    -    // TODO in future support policies etc
    +    /** As {@link #lookup(Predicate)} comparing the ID of the object with the given string
*/
         public <T extends BrooklynObject> T lookup(String id, Class<T> type);

     
    -}
    \ No newline at end of file
    +    /** Finds a {@link BrooklynObject} known in this management context 
    +     * satisfying the given predicate, or null */
    +    public <T extends BrooklynObject> T lookup(Predicate<? super T> filter);
    --- End diff --
    
    I'm hesitant about adding this, but see why you're doing it (because it fits with the
existing `lookup` naming, and is useful for you're use-case!).
    
    I don't want to bloat the public `ManagementContext` api. In other places, we've grouped
related methods (e.g. `entity.config().set(...)`, rather than `entity.setConfig(...)`). Perhaps
we should group the lookup methods.
    
    I also prefer the guava conventions, rather than returning null when not found. Should
we go for `Maybe<T> tryLookup(Predicate<? super T> filter)`, or even `tryFind()`?
That is different from the existing `lookup(String id)`, but is a better api and is consistent
with lots of other parts of Brooklyn.


---

Mime
View raw message