[GitHub] [storm] 6harat commented on pull request #3546: [STORM-3924] Support for declaring WorkerHook in Flux topology definitions

["6harat (via GitHub)" <gi...@apache.org>]

6harat commented on PR #3546:
URL: https://github.com/apache/storm/pull/3546#issuecomment-1605932509

   > I was going through other parts of the code and found this TODO. Seems like it was intended to be connected to WorkerHooks when those were first introduced.
   > 
   > ```
   >     private Map<String, Object> makeUserResources() {
   >         /* TODO: need to invoke a hook provided by the topology, giving it a chance to create user resources.
   >          * this would be part of the initialization hook
   >          * need to separate workertopologycontext into WorkerContext and WorkerUserContext.
   >          * actually just do it via interfaces. just need to make sure to hide setResource from tasks
   >          */
   >         return new HashMap<>();
   >     }
   > ```
   > 
   > AFAIS, the current implementation will always lead to empty userResources as no interface exposes a way to allow user to set them.
   > 
   > Lmk, if my above understanding is correct and this change can also be taken up. this would allow for better application context management. Will put this fix in a separate PR.
   > 
   > cc: @abhishekagarwal87 , @schonfeld , @revans2 , @nathanmarz , @rzo1 , @bipinprasad
   
   this has been taken up here: https://github.com/apache/storm/pull/3547


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@storm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org