bipinprasad commented on a change in pull request #3274:
URL: https://github.com/apache/storm/pull/3274#discussion_r491192564



##########
File path: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
##########
@@ -814,7 +814,12 @@ private static int numUsedWorkers(SchedulerAssignment 
assignment) {
 
     private boolean auditAssignmentChanges(Map<String, Assignment> 
existingAssignments,
                                            Map<String, Assignment> 
newAssignments) {
-        assert existingAssignments != null && newAssignments != null;
+        if (existingAssignments == null) {

Review comment:
       Correct, but only in test. This is fairly standard use of assert in 
test. But putting this in main code is not. 
   
   If it a legitimate error condition then it should be checked. If the test 
needs to check something, then it should be in the test class.
   Otherwise, it is gives the wrong impression that the assert is checking 
something at runtime, when it typically isn't.
   
   That is the premise of this change.




----------------------------------------------------------------
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]


Reply via email to