You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by "Patrick Hunt (JIRA)" <ji...@apache.org> on 2009/05/18 19:50:45 UTC
[jira] Created: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
-------------------------------------------------------------------------------
Key: ZOOKEEPER-407
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
Project: Zookeeper
Issue Type: Sub-task
Components: server
Reporter: Patrick Hunt
Assignee: Flavio Paiva Junqueira
Fix For: 3.2.0
Flavio please address the findbugs warnings in quorum and sub packages. See
http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12710794#action_12710794 ]
Flavio Paiva Junqueira commented on ZOOKEEPER-407:
--------------------------------------------------
For this patch I'm certainly not proposing that we fix any problem with exit calls. All modifications I have suggested are to get rid of the findbugs warnings. In fact, my preference is to leave the exit calls as they are and perhaps deal with them in a separate jira if there is any need to do so.
> address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
> -------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-407
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
> Project: Zookeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Patrick Hunt
> Assignee: Flavio Paiva Junqueira
> Fix For: 3.2.0
>
> Attachments: ZOOKEEPER-407.patch, ZOOKEEPER-407.patch
>
>
> Flavio please address the findbugs warnings in quorum and sub packages. See
> http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
> for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
> In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
> to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Paiva Junqueira updated ZOOKEEPER-407:
---------------------------------------------
Attachment: ZOOKEEPER-407.patch
I'm not 100% sure of what to do with the System.exit calls sprinkled all over the code. I'm including some modifications in this patch, but I'm afraid they won't replicate the behavior we have currently. In some cases, we might consider throwing an exception and let some method higher up in the call path decide the appropriate measure. In other cases, like the one of LeaderElection, I'm not sure it is really necessary to exit if we can't open the socket. I understand it won't work, but perhaps throwing an exception again or simply logging a message might do the job. In any case, I have marked the occurrences for this package with a "TODO".
I would appreciate some comments on this matter.
> address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
> -------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-407
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
> Project: Zookeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Patrick Hunt
> Assignee: Flavio Paiva Junqueira
> Fix For: 3.2.0
>
> Attachments: ZOOKEEPER-407.patch, ZOOKEEPER-407.patch
>
>
> Flavio please address the findbugs warnings in quorum and sub packages. See
> http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
> for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
> In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
> to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated ZOOKEEPER-407:
------------------------------------
Hadoop Flags: [Reviewed]
+1 ready to commit
> address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
> -------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-407
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
> Project: Zookeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Patrick Hunt
> Assignee: Flavio Paiva Junqueira
> Fix For: 3.2.0
>
> Attachments: ZOOKEEPER-407.patch, ZOOKEEPER-407.patch, ZOOKEEPER-407.patch
>
>
> Flavio please address the findbugs warnings in quorum and sub packages. See
> http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
> for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
> In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
> to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
Posted by "Benjamin Reed (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Benjamin Reed updated ZOOKEEPER-407:
------------------------------------
Resolution: Fixed
Status: Resolved (was: Patch Available)
Committed revision 776826.
> address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
> -------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-407
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
> Project: Zookeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Patrick Hunt
> Assignee: Flavio Paiva Junqueira
> Fix For: 3.2.0
>
> Attachments: ZOOKEEPER-407.patch, ZOOKEEPER-407.patch, ZOOKEEPER-407.patch
>
>
> Flavio please address the findbugs warnings in quorum and sub packages. See
> http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
> for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
> In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
> to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Paiva Junqueira updated ZOOKEEPER-407:
---------------------------------------------
Attachment: ZOOKEEPER-407.patch
Addressing problems pointed by findbugs. Some of them are not trivial, so I would appreciate a thorough review of this patch.
> address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
> -------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-407
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
> Project: Zookeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Patrick Hunt
> Assignee: Flavio Paiva Junqueira
> Fix For: 3.2.0
>
> Attachments: ZOOKEEPER-407.patch
>
>
> Flavio please address the findbugs warnings in quorum and sub packages. See
> http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
> for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
> In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
> to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12710787#action_12710787 ]
Patrick Hunt commented on ZOOKEEPER-407:
----------------------------------------
Take a look at what I did in ZOOKEEPER-410, I basically allowed a couple of exceptions where I thought:
1) big impact if we didn't allow (potential instability/incorrectness in particular)
2) I thought it was ok to exit if we; clearly documented and esp logged as FATAL the cause
3) command line driver code (ie main()) is ok to use exit
it's not clear to me that in server code (vs library) exit should be entirely disallowed. I'm a follower of the
"fail fast" approach - I'd rather see us exit with useful feedback than linger. In library code obv you don't
want to exit as this may not be what all clients want to do - but for servers I think it's ok, esp if we review/approve
each exception (which we already do).
> address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
> -------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-407
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
> Project: Zookeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Patrick Hunt
> Assignee: Flavio Paiva Junqueira
> Fix For: 3.2.0
>
> Attachments: ZOOKEEPER-407.patch, ZOOKEEPER-407.patch
>
>
> Flavio please address the findbugs warnings in quorum and sub packages. See
> http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
> for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
> In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
> to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12711892#action_12711892 ]
Hudson commented on ZOOKEEPER-407:
----------------------------------
Integrated in ZooKeeper-trunk #321 (See [http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/321/])
> address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
> -------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-407
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
> Project: Zookeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Patrick Hunt
> Assignee: Flavio Paiva Junqueira
> Fix For: 3.2.0
>
> Attachments: ZOOKEEPER-407.patch, ZOOKEEPER-407.patch, ZOOKEEPER-407.patch
>
>
> Flavio please address the findbugs warnings in quorum and sub packages. See
> http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
> for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
> In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
> to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Paiva Junqueira updated ZOOKEEPER-407:
---------------------------------------------
Status: Patch Available (was: Open)
> address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
> -------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-407
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
> Project: Zookeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Patrick Hunt
> Assignee: Flavio Paiva Junqueira
> Fix For: 3.2.0
>
> Attachments: ZOOKEEPER-407.patch, ZOOKEEPER-407.patch, ZOOKEEPER-407.patch
>
>
> Flavio please address the findbugs warnings in quorum and sub packages. See
> http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
> for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
> In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
> to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-407) address all findbugs warnings in
org.apache.zookeeper.server.quorum.** packages
Posted by "Flavio Paiva Junqueira (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-407?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Flavio Paiva Junqueira updated ZOOKEEPER-407:
---------------------------------------------
Attachment: ZOOKEEPER-407.patch
Added System.exit exceptions to exclude file.
> address all findbugs warnings in org.apache.zookeeper.server.quorum.** packages
> -------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-407
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-407
> Project: Zookeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Patrick Hunt
> Assignee: Flavio Paiva Junqueira
> Fix For: 3.2.0
>
> Attachments: ZOOKEEPER-407.patch, ZOOKEEPER-407.patch, ZOOKEEPER-407.patch
>
>
> Flavio please address the findbugs warnings in quorum and sub packages. See
> http://hudson.zones.apache.org/hudson/view/ZooKeeper/job/ZooKeeper-trunk/308/violations/
> for a list of issues or use ant's findbugs (or eclipse) to identify the issues.
> In general we should try to fix these, but ignoring the warning (annotation) is ok if it includes comments to effect why, also has
> to be reviewed/approved.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.