Github user revans2 commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2319#discussion_r138659373
  
    --- Diff: storm-core/src/clj/org/apache/storm/cluster.clj ---
    @@ -244,15 +255,19 @@
     
     ;; Watches should be used for optimization. When ZK is reconnecting, 
they're not guaranteed to be called.
     (defnk mk-storm-cluster-state
    -  [cluster-state-spec :acls nil :context (ClusterStateContext.)]
    -  (let [[solo? cluster-state] (if (instance? ClusterState 
cluster-state-spec)
    -                                [false cluster-state-spec]
    -                                [true (mk-distributed-cluster-state 
cluster-state-spec :auth-conf cluster-state-spec :acls acls :context context)])
    +  [conf :cluster-state nil :acls nil :context (ClusterStateContext.) 
:backend nil]
    +  (let [[solo? cluster-state] (if (and (not-nil? cluster-state) (instance? 
ClusterState cluster-state))
    +                                [false cluster-state]
    +                                [true (mk-distributed-cluster-state conf 
:auth-conf conf :acls acls :context context)])
    +        assignments-backend (if (nil? backend)
    +                              (doto (InMemoryAssignmentBackend.) (.prepare 
nil nil))
    --- End diff --
    
    Why do we call `(.prepare nil nil)` only on the InMemoryAssignmentBackend?  
It feels like we might be making it difficult to extend it in the future.  From 
what I have seen the assignments dir is a subdirectory of storm-local.  Could 
we just make prepare take the conf, and then we can pass in the conf we already 
have.


---

Reply via email to