tisonkun commented on code in PR #430:
URL: https://github.com/apache/curator/pull/430#discussion_r974852099


##########
curator-recipes/src/main/java/org/apache/curator/framework/recipes/leader/LeaderLatch.java:
##########
@@ -190,6 +190,12 @@ public void close() throws IOException
         close(closeMode);
     }
 
+    // for testing
+    void closeOnDemand() throws IOException
+    {
+        internalClose(closeMode, false);
+    }

Review Comment:
   I think it's a design issue we can resolve in another thread. Generally, I'd 
prefer `close` to be idempotent but the original code isn't. Your test case can 
fail if:
   
   1. Checking state != CLOSE.
   2. Internally closed.
   3. Outside closed.
   
   And it happens in the CI environment once.
   
   Your suggestion can be reasonable but I don't want to spend too much time on 
this bug fix PR. A package-level visible method is for internal usage :)



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

To unsubscribe, e-mail: dev-unsubscr...@curator.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to