Refactor Optional parameters in the constructor of strategy #836
Conversation
Pull latest
vmaheshw
requested review from
jzakaryan,
somandal,
atoomula and
shrinandthakkar
| createStickyPartitionAssignmentStrategyObject(0, 0, null, _clusterName); | ||
| } | ||
|
|
||
| private void createStickyPartitionAssignmentStrategyObject(int partitionsPerTask, int partitionFullnessFactorPct, |
There was a problem hiding this comment.
nit: rather than adding this extra function, can't you just directly call createStickyPartitionAssignmentStrategy with "enableElasticTask" set to 'false' and just ignore the returned assignment?
There was a problem hiding this comment.
Created a default to avoid modifying some of the defaults everywhere. It was easier to refactor and fix at one place.
There was a problem hiding this comment.
nit: can you rename this function to either be the same as createStickyPartitionAssignmentStrategy or createStickyPartitionAssignmentStrategyWithElasticTaskDisabled?
...ver/src/test/java/com/linkedin/datastream/server/assignment/TestStickyMulticastStrategy.java
Outdated
Show resolved
Hide resolved
...ver/src/test/java/com/linkedin/datastream/server/assignment/TestStickyMulticastStrategy.java
Show resolved
Hide resolved
.../src/main/java/com/linkedin/datastream/server/assignment/StickyMulticastStrategyFactory.java
Outdated
Show resolved
Hide resolved
...st/java/com/linkedin/datastream/server/assignment/TestPartitionAssignmentStrategyConfig.java
Outdated
Show resolved
Hide resolved
...om/linkedin/datastream/server/assignment/TestLoadBasedPartitionAssignmentStrategyConfig.java
Show resolved
Hide resolved
| createStickyPartitionAssignmentStrategyObject(0, 0, null, _clusterName); | ||
| } | ||
|
|
||
| private void createStickyPartitionAssignmentStrategyObject(int partitionsPerTask, int partitionFullnessFactorPct, |
There was a problem hiding this comment.
nit: can you rename this function to either be the same as createStickyPartitionAssignmentStrategy or createStickyPartitionAssignmentStrategyWithElasticTaskDisabled?
…#836) There are lots of Optional fields added to the constructor of the strategy. Most of these fields are not really used as Optional. Refactoring the code to make it simpler and easier to maintain. We will make the fields Optional in future on need basis.
There are lots of Optional fields added to the constructor of the strategy. Most of these fields are not really used as Optional. Refactoring the code to make it simpler and easier to maintain. We will make the fields Optional in future on need basis.