mynameborat commented on a change in pull request #1421:
URL: https://github.com/apache/samza/pull/1421#discussion_r476728011
##########
File path:
samza-core/src/main/java/org/apache/samza/clustermanager/ContainerManager.java
##########
@@ -80,19 +82,23 @@
private final Optional<StandbyContainerManager> standbyContainerManager;
+ private final LocalityManager localityManager;
Review comment:
1. `SamzaApplicationState` is a holder class to share state between
components and not supposed to hold on any manager classes. We have a TODO to
remove JobModelManager out of it. It would be counter productive to introduce
another component into the holder class.
2. I am not a huge fan of Static for following reasons
- It makes testing harder and using power mock for component testing
everywhere.
- Utility classes that handles components with lifecycle is harder to reason
about.
- Static methods are harder to evolve/extend without breaking the signature
& modifying the callers and forces the dependencies it uses to be either passed
as arguments or define as static variables within the class.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
