azagrebin commented on issue #8416: [FLINK-12331] Introduce partition/gate setup to decouple task registration with NetworkEnvironment URL: https://github.com/apache/flink/pull/8416#issuecomment-492193232 Thanks for more review @zhijiangW ! I removed `PartitionBufferPoolFactory` commits and changed them to function suppliers. Normal partition/gate factories are now responsible for buffer pool creation logic. We can reintroduce `PartitionBufferPoolFactory` in future if normal factories get too many responsibilities. True,`ShuffleService` will be a factory for partitions/gates. I just do not want it to take too many responsibilities. I think delegating further creation process to concrete factories already makes sense atm. Ideally, any class should not take more than one responsibility among couple of abstraction layers but it also depends practically on how it is already big or not, of course. Also class constructors should ideally not have any complex logic to see real class dependencies and simplify dependency injection, e.g. for testing. We can just have `BasePartitionFactory` and `FSResultPartitionFactory` for `FSResultPartition` then or see how it goes.
---------------------------------------------------------------- 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] With regards, Apache Git Services
