bipinprasad commented on a change in pull request #3274:
URL: https://github.com/apache/storm/pull/3274#discussion_r435431019
##########
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:
Definitely need critical review of code that is throwing unchecked
exception (including AssertionError()) and look for options - or complete
removal - even in the changed code.
At best, "assert" usage should be viewed as lazy coding in private methods.
And should not be used in protected or public methods.
https://docs.oracle.com/javase/7/docs/technotes/guides/language/assert.html
(note the language that it should not be used in public methods).
This does not mean that private methods cannot throw Exceptions. They can
and very often do. But those are checked exceptions (i.e. present in method
signature).
The goal of this change is to remove asserts. Replace with proper checking
where possible, throw checked Exception where appropriate. Reason is quite
straightforward - assert statements may not be executed and gives a false
impression that something is being done.
----------------------------------------------------------------
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:
us...@infra.apache.org
