[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12714407#action_12714407 ] Hadoop QA commented on ZOOKEEPER-358: - +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12409362/ZOOKEEPER-358.patch against trunk revision 779716. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/94/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/94/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/94/console This message is automatically generated. Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Fix For: 3.2.0 Attachments: ZOOKEEPER-358.patch, ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12714312#action_12714312 ] Mahadev konar commented on ZOOKEEPER-358: - flavio, was this patch included in ZOOKEEPER-383? Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Attachments: ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12711299#action_12711299 ] Benjamin Reed commented on ZOOKEEPER-358: - +1 looks good Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Attachments: ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12710125#action_12710125 ] Flavio Paiva Junqueira commented on ZOOKEEPER-358: -- Thanks, Mahadev. It sounds right that APIs often enough would throw an exception and perhaps this is the behavior a typical java developer would expect. I'm fine either way, and I was just trying to understand if there the reason for throwing an exception in this case is just a matter of taste or if there is something more fundamental behind. I was thinking about perhaps incorporating the patch for this issue into ZOOKEEPER-383. Is this a good idea? Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Attachments: ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12710134#action_12710134 ] Mahadev konar commented on ZOOKEEPER-358: - flavio, Yeah I think ZOOKEEPER-383 and this jira are related. So its fine if you want to incorporate this patch into ZOOKEEPER-383. In case you do that please make sure that you link this issue to ZOOKEEPER-383 saying that this patch is incorporated in ZOOKEEPER-383. Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Attachments: ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12709767#action_12709767 ] Flavio Paiva Junqueira commented on ZOOKEEPER-358: -- Thinking more carefully about this modification, I wonder if it really makes sense to throw an exception if you try to open a ledger that doesn't exist. Currently, the code of openLedger returns null if the id passed does not exist. I'm questioning the validity of the modification because I have been working on a patch to make create, open, and close operations asynchronous, and with asynchronous operations we cannot really throw an exception. In summary, I think I prefer the current approach (the one in trunk) because we'll have to do something similar with an asynchronous version of open. That is, we'll indicate that an attempt to open a non-existing ledger is invalid through a null ledger handle or through a return code value. Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Attachments: ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12709910#action_12709910 ] Mahadev konar commented on ZOOKEEPER-358: - flavio, We should still throw an exception with the openledger call. The asynchronous api is no excuse to hide excpetions. THe idea is to have asycnrhonous calls return the same error codes as the exception that you throw. For an example, take a look at zookeeper client api in java. It has both the synchronous calls and aysnc calls. The sync calls throw an exception and the async calls call the callback function with a return code which maps to the exception thrown via sync calls. Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Attachments: ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12709496#action_12709496 ] Mahadev konar commented on ZOOKEEPER-358: - falvio, adding a test would a good thing so that it prevents breaking the code later. also, specially in this case, since its a api semantics change, also a test for this should nto be mroe than 2 lines of code? :) Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Attachments: ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12708203#action_12708203 ] Hadoop QA commented on ZOOKEEPER-358: - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12407766/ZOOKEEPER-358.patch against trunk revision 772843. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no tests are needed for this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/68/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/68/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/68/console This message is automatically generated. Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Attachments: ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-358) Throw exception when ledger does not exist
[ https://issues.apache.org/jira/browse/ZOOKEEPER-358?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=12708207#action_12708207 ] Flavio Paiva Junqueira commented on ZOOKEEPER-358: -- I don't think this patch requires a new test or modifications to existing ones. Any objection? Throw exception when ledger does not exist -- Key: ZOOKEEPER-358 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-358 Project: Zookeeper Issue Type: Improvement Components: contrib-bookkeeper Affects Versions: 3.1.1 Reporter: Luca Telloli Assignee: Flavio Paiva Junqueira Priority: Minor Attachments: ZOOKEEPER-358.patch Currently, openLedger() in the BookKeeper client returns null if the ledger ID does not exist on ZK. Maybe it would be better to throw a specific exception so it can be handled by the client side. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
