[
https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135574#comment-13135574
]
[email protected] commented on BOOKKEEPER-89:
---------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2543/#review2843
-----------------------------------------------------------
Thanks a lot for working on this patch, Ivan. It is mostly good for me, but I
have a few comments if you don't mind.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
<https://reviews.apache.org/r/2543/#comment6370>
We differentiate between password and ledger key in BookKeeper. The
password is the string that the application passes to bookkeeper when creating
a ledger. The password is used to generate the ledger key (check the
constructor of ledger handle) and the mac key when MAC is being used.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java
<https://reviews.apache.org/r/2543/#comment6376>
I understand that these tools need visibility to some internal methods of
the client package, but this class is not really part of the bookkeeper client.
It is instead a tool for things like bookie recovery.
I was wondering if we should at least have it in a separate folder to avoid
confusion.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java
<https://reviews.apache.org/r/2543/#comment6371>
Is this a tab?
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java
<https://reviews.apache.org/r/2543/#comment6373>
Why are we moving this test to a different package?
- fpj
On 2011-10-24 14:53:25, Ivan Kelly wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2543/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-24 14:53:25)
bq.
bq.
bq. Review request for bookkeeper.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Changes are as follows.
bq.
bq. BookKeeper#createLedger, parameter is named passwd, "Key" used in
LedgerHandle api
bq. BookKeeper#getBookieClient shouldn't be public
bq. BookKeeper#createComplete shouldn't be public
bq. BookKeeper#openComplete shouldn't be public
bq. BookKeeper#deleteComplete shouldn't be public
bq. BookKeeper#halt could be changed to close(), should throw a BKException
bq.
bq. LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be
private
bq. LedgerHandle#getLedgerMetadata shouldn't be public
bq. LedgerHandle#getDigestManager shouldn't be public
bq. LedgerHandle#getDistributionSchedule shouldn't be public
bq. LedgerHandle#writeLedgerConfig shouldn't be public
bq. LedgerHandle#addEntry should return void, errors should go in an Exception
bq. LedgerHandle#readComplete should not be public
bq. LedgerHandle#addComplete should not be public
bq. LedgerHandle#readLastConfirmedCompelte should not be public
bq. LedgerHandle#closeComplete should not be public
bq.
bq. ASyncCallback#RecoverCallback shouldn't be public
bq.
bq.
bq. This addresses bug BOOKKEEPER-89.
bq. https://issues.apache.org/jira/browse/BOOKKEEPER-89
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/AsyncCallback.java
6421460
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
6af43ae
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java
PRE-CREATION
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DigestManager.java
d4af3fa
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DistributionSchedule.java
78aaa15
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
959df73
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/MacDigestManager.java
1131652
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/tools/BookKeeperTools.java
94e444c
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTestClient.java
dfc63d7
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java
PRE-CREATION
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java
224c796
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieFailureTest.java
82483f3
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieReadWriteTest.java
56331ef
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieRecoveryTest.java
f933ba1
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/LedgerCacheTest.java
3a78507
bq.
hedwig-server/src/main/java/org/apache/hedwig/server/benchmark/BookkeeperBenchmark.java
a934985
bq.
hedwig-server/src/main/java/org/apache/hedwig/server/netty/PubSubServer.java
726341d
bq.
hedwig-server/src/test/java/org/apache/hedwig/server/persistence/BookKeeperTestBase.java
b918d97
bq. pom.xml 2392db5
bq.
bq. Diff: https://reviews.apache.org/r/2543/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Ivan
bq.
bq.
> Bookkeeper API changes for initial Bookkeeper release
> -----------------------------------------------------
>
> Key: BOOKKEEPER-89
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-89
> Project: Bookkeeper
> Issue Type: Improvement
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Fix For: 4.0.0
>
> Attachments: BOOKKEEPER-89.diff, BOOKKEEPER-89.diff
>
>
> Changes are as follows.
> BookKeeper#createLedger, parameter is named passwd, "Key" used in
> LedgerHandle api
> BookKeeper#getBookieClient shouldn't be public
> BookKeeper#createComplete shouldn't be public
> BookKeeper#openComplete shouldn't be public
> BookKeeper#deleteComplete shouldn't be public
> BookKeeper#halt could be changed to close(), should throw a BKException
> LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be
> private
> LedgerHandle#getLedgerMetadata shouldn't be public
> LedgerHandle#getDigestManager shouldn't be public
> LedgerHandle#getDistributionSchedule shouldn't be public
> LedgerHandle#writeLedgerConfig shouldn't be public
> LedgerHandle#addEntry should return void, errors should go in an Exception
> LedgerHandle#readComplete should not be public
> LedgerHandle#addComplete should not be public
> LedgerHandle#readLastConfirmedCompelte should not be public
> LedgerHandle#closeComplete should not be public
> ASyncCallback#RecoverCallback shouldn't be public
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
