NealSun96 commented on a change in pull request #973:
URL: https://github.com/apache/helix/pull/973#discussion_r420966673
##########
File path:
helix-core/src/main/java/org/apache/helix/task/TaskStateModelFactory.java
##########
@@ -81,6 +79,12 @@ public Thread newThread(Runnable r) {
@Override
public TaskStateModel createNewStateModel(String resourceName, String
partitionKey) {
+ if (_taskExecutor == null) {
Review comment:
With the recent changes, `initializeTaskMonitor()` is called by both
constructors so it makes sense for it to be factored out. I'm not entirely
convinced that it's correct to put all logic in one function - I always thought
it's a good practice to break functions down when necessary into logical chunks
to avoid big messy functions.
Since this section of code is outdated, we can discuss in the new
conversation if necessary.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
