[
https://issues.apache.org/jira/browse/BOOKKEEPER-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13135805#comment-13135805
]
[email protected] commented on BOOKKEEPER-89:
---------------------------------------------------------
bq. On 2011-10-26 00:03:20, fpj wrote:
bq. >
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java,
line 187
bq. > <https://reviews.apache.org/r/2543/diff/2/?file=52724#file52724line187>
bq. >
bq. > 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.
Ah, I'll change this back then and update the javadoc for
LedgerHandle#getLedgerKey
bq. On 2011-10-26 00:03:20, fpj wrote:
bq. >
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java,
line 547
bq. > <https://reviews.apache.org/r/2543/diff/2/?file=52725#file52725line547>
bq. >
bq. > Is this a tab?
Nope, it's line which contains only whitespace, in this case spaces. Its
considered bad style, so thats probably why it's on show here.
bq. On 2011-10-26 00:03:20, fpj wrote:
bq. >
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/LedgerCacheTest.java,
line 22
bq. > <https://reviews.apache.org/r/2543/diff/2/?file=52732#file52732line22>
bq. >
bq. > Why are we moving this test to a different package?
It accesses MacDigestManager, which is now package private. In general I think
it's better to have tests in the same package as the code they're testing, so
that they can poke at internals without having to open them to the world.
bq. On 2011-10-26 00:03:20, fpj wrote:
bq. >
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperTools.java,
line 1
bq. > <https://reviews.apache.org/r/2543/diff/2/?file=52725#file52725line1>
bq. >
bq. > 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.
bq. >
bq. > I was wondering if we should at least have it in a separate folder
to avoid confusion.
It's a tricky one. While it is toollike, it reaches into the internals of the
client code. Really a tool should just use the public api itself. How about
this? I'll move the BookKeeperTools back to tools, and put the main() in it,
but then I'll create a class BookKeeperAdmin which acts as an admin client and
implements the code that reaches into the internals. Then BookKeeperTools can
use the public methods exposed by BookKeeperAdmin.
- Ivan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2543/#review2843
-----------------------------------------------------------
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
