Skip to content

Issue #1195 Handle optional fields gracefully in ReadExplicitLac response#1196

Closed
jvrao wants to merge 3 commits into
apache:masterfrom
jvrao:bk-issue-1195-lacfix
Closed

Issue #1195 Handle optional fields gracefully in ReadExplicitLac response#1196
jvrao wants to merge 3 commits into
apache:masterfrom
jvrao:bk-issue-1195-lacfix

Conversation

@jvrao
Copy link
Copy Markdown
Contributor

@jvrao jvrao commented Feb 22, 2018

Descriptions of the changes in this PR:

ExplicitLac and header of the last entry are optional fields in the protocol.
Handle them as optionals in the response processing.

TestPendingReadLacOp.java test added by: Samuel Just sjust@salesforce.com

Signed-off-by: Venkateswararao Jujjuri (JV) vjujjuri@salesforce.com
Signed-off-by: Samuel Just sjust@salesforce.com

Master Issue: #1195


Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

If this PR is a BookKeeper Proposal (BP):

  • Make sure the PR title is formatted like:
    <BP-#>: Description of bookkeeper proposal
    e.g. BP-1: 64 bits ledger is support
  • Attach the master issue link in the description of this PR.
  • Attach the google doc link if the BP is written in Google Doc.

Otherwise:

  • Make sure the PR title is formatted like:
    <Issue #>: Description of pull request
    e.g. Issue 123: Description ...
  • Make sure tests pass via mvn clean apache-rat:check install spotbugs:check.
  • Replace <Issue #> in the title with the actual Issue number.

…c response

ExplicitLac and header of the last entry are optional fields in the protocol.
Handle them as optionals in the response processing.

TestPendingReadLacOp.java test added by: Samuel Just <sjust@salesforce.com>

Signed-off-by: Venkateswararao Jujjuri (JV) <vjujjuri@salesforce.com>
Signed-off-by: Samuel Just <sjust@salesforce.com>
@sijie sijie changed the title Issue #1195 Handle optional fields gracefully in ReadExplicitLac resp… Issue #1195 Handle optional fields gracefully in ReadExplicitLac response Feb 22, 2018
@sijie
Copy link
Copy Markdown
Member

sijie commented Feb 22, 2018

@jvrao these two checkstyle violations need to be addressed.

[INFO] Starting audit...
[ERROR] /Users/travis/build/apache/bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java:1453: Javadoc comment at column 4 has parse error. Unrecognized error from ANTLR parser: null [JavadocParagraph]
[ERROR] /Users/travis/build/apache/bookkeeper/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestPendingReadLacOp.java:25:8: Unused import: io.netty.buffer.ByteBuf. [UnusedImports]
Audit done.

Signed-off-by: Venkateswararao Jujjuri (JV) <vjujjuri@salesforce.com>
@sijie
Copy link
Copy Markdown
Member

sijie commented Feb 23, 2018

It seems oraclejdk9 is complaining Integer(int) has been deprecated

https://travis-ci.org/apache/bookkeeper/jobs/345375922

[INFO] -------------------------------------------------------------
[WARNING] COMPILATION WARNING : 
[INFO] -------------------------------------------------------------
[WARNING] /home/travis/build/apache/bookkeeper/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestPendingReadLacOp.java:[58,43] Integer(int) in java.lang.Integer has been deprecated
[WARNING] /home/travis/build/apache/bookkeeper/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestPendingReadLacOp.java:[94,43] Integer(int) in java.lang.Integer has been deprecated
[INFO] 2 warnings 
[INFO] -------------------------------------------------------------
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /home/travis/build/apache/bookkeeper/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestPendingReadLacOp.java: warnings found and -Werror specified

@Override
public void initiate() {
for (int i = 0; i < lh.metadata.currentEnsemble.size(); i++) {
final Integer index = new Integer(i);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you just pass "i" in readLacComplate?

@sijie sijie added this to the 4.7.0 milestone Feb 26, 2018
@sijie sijie closed this in 8f6373e Feb 26, 2018
sijie pushed a commit that referenced this pull request Feb 26, 2018
…onse

Descriptions of the changes in this PR:

ExplicitLac and header of the last entry are optional fields in the protocol.
Handle them as optionals in the response processing.

TestPendingReadLacOp.java test added by: Samuel Just <sjustsalesforce.com>

Signed-off-by: Venkateswararao Jujjuri (JV) <vjujjurisalesforce.com>
Signed-off-by: Samuel Just <sjustsalesforce.com>

Master Issue: #1195

Author: JV Jujjuri <vjujjuri@salesforce.com>
Author: JV <vjujjuri@salesforce.com>

Reviewers: Sijie Guo <sijie@apache.org>

This closes #1196 from jvrao/bk-issue-1195-lacfix, closes #1195

(cherry picked from commit 8f6373e)
Signed-off-by: Sijie Guo <sijie@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants