[GitHub] rafaelweingartner commented on a change in pull request #837: CLOUDSTACK-8855 Improve Error Message for Host Alert State and reconnect host API.

[gi...@git.apache.org]

rafaelweingartner commented on a change in pull request #837: CLOUDSTACK-8855 Improve Error Message for Host Alert State and reconnect host API.
URL: https://github.com/apache/cloudstack/pull/837#discussion_r120749032
 
 

 ##########
 File path: engine/components-api/src/com/cloud/agent/AgentManager.java
 ##########
 @@ -141,7 +142,7 @@
 
     public void pullAgentOutMaintenance(long hostId);
 
-    boolean reconnect(long hostId);
+    void reconnect(long hostId) throws CloudRuntimeException, AgentUnavailableException;
 
 Review comment:
   This explanation that you just said, for me, seems a great piece of information to be in a method documentation. Therefore, instead of declaring a runtime, we can document why we need to catch and deal with it, something in the JavaDoc explaining that would be great; e.g."hey dude, if you want to use this method, please catch and deal with CloudRuntimeExceptions" (of couse, with a more polished language ;) ).
   
   I am still not convinced on declaring runtime exceptions, though. Philosophically speaking, they do not need to (should not?) be declared. It does not harm, but I also do not see the benefit of it. 
   
   Anyways, if people are comfortable with it, I cannot do anything else.
   
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services