[GitHub] [cassandra] yifan-c commented on a change in pull request #729: CASSANDRA-15954: Include taskId in the compactionInfo toString

[GitBox <gi...@apache.org>]

yifan-c commented on a change in pull request #729:
URL: https://github.com/apache/cassandra/pull/729#discussion_r483777159



##########
File path: src/java/org/apache/cassandra/db/compaction/CompactionInfo.java
##########
@@ -132,6 +132,13 @@ public String toString()
         StringBuilder buff = new StringBuilder();
         buff.append(getTaskType());
 
+        if (getTaskId() != null)
+        {
+            buff.append('(')
+                .append(getTaskId())
+                .append(')');
+        }

Review comment:
       Double checked the call sites. I agree with out that `getTaskId` cannot return null in the current code base. 
   Having the null check here mainly to be consistent with the existing output style (wrap with parenthesis only when id is present) and the null check at [CompactionManager.java#L2025-L2026](https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/compaction/CompactionManager.java#L2025-L2026)
   
   I am OK to just remove the null check. As you mentioned, it does no harm even with its value being null. 
   
   (Maybe the constructor should assert the task ID not to be null. but it is not related to this ticket.)




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org