HBase
  1. HBase
  2. HBASE-5591

ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.95.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Activity

      Hide
      Phabricator added a comment -

      sc requested code review of "HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()".
      Reviewers: tedyu, dhruba, JIRA

      ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()

      Remove the redundant method.

      Task ID: #

      Blame Rev:

      TEST PLAN
      Revert Plan:

      Tags:

      REVISION DETAIL
      https://reviews.facebook.net/D2355

      AFFECTED FILES
      src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java
      src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java

      MANAGE HERALD DIFFERENTIAL RULES
      https://reviews.facebook.net/herald/view/differential/

      WHY DID I GET THIS EMAIL?
      https://reviews.facebook.net/herald/transcript/5229/

      Tip: use the X-Herald-Rules header to filter Herald messages in your client.

      Show
      Phabricator added a comment - sc requested code review of " HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()". Reviewers: tedyu, dhruba, JIRA ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes() Remove the redundant method. Task ID: # Blame Rev: TEST PLAN Revert Plan: Tags: REVISION DETAIL https://reviews.facebook.net/D2355 AFFECTED FILES src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/5229/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
      Hide
      Phabricator added a comment -

      tedyu has accepted the revision "HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()".

      If tests pass.

      REVISION DETAIL
      https://reviews.facebook.net/D2355

      BRANCH
      getbytes

      Show
      Phabricator added a comment - tedyu has accepted the revision " HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()". If tests pass. REVISION DETAIL https://reviews.facebook.net/D2355 BRANCH getbytes
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12518598/HBASE-5591.D2355.1.patch
      against trunk revision .

      +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 new tests are needed for this patch.
      Also please list what manual steps were performed to verify 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 appears to introduce 1 new Findbugs (version 1.3.9) warnings.

      +1 release audit. The applied patch does not increase the total number of release audit warnings.

      -1 core tests. The patch failed these unit tests:
      org.apache.hadoop.hbase.mapreduce.TestImportTsv
      org.apache.hadoop.hbase.mapred.TestTableMapReduce
      org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat

      Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1261//testReport/
      Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1261//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1261//console

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12518598/HBASE-5591.D2355.1.patch against trunk revision . +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 new tests are needed for this patch. Also please list what manual steps were performed to verify 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 appears to introduce 1 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.mapreduce.TestImportTsv org.apache.hadoop.hbase.mapred.TestTableMapReduce org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1261//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1261//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1261//console This message is automatically generated.
      Hide
      Phabricator added a comment -

      stack has commented on the revision "HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()".

      Otherwise patch looks good.

      INLINE COMMENTS
      src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java:611 Why do you think this comment is here Scott? Its ominous lookinh.

      REVISION DETAIL
      https://reviews.facebook.net/D2355

      BRANCH
      getbytes

      Show
      Phabricator added a comment - stack has commented on the revision " HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()". Otherwise patch looks good. INLINE COMMENTS src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java:611 Why do you think this comment is here Scott? Its ominous lookinh. REVISION DETAIL https://reviews.facebook.net/D2355 BRANCH getbytes
      Hide
      Ted Yu added a comment -

      Integrated to trunk.

      Show
      Ted Yu added a comment - Integrated to trunk.
      Hide
      Phabricator added a comment -

      sc has commented on the revision "HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()".

      INLINE COMMENTS
      src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java:611 It was added by me actually.

      Because I checked Bytes.toBytes(ByteBuffer bb) and found that it's different from this one.

      But this one is the same as Bytes.getBytes(ByteBuffer bb).

      These names are really confusing.

      REVISION DETAIL
      https://reviews.facebook.net/D2355

      BRANCH
      getbytes

      Show
      Phabricator added a comment - sc has commented on the revision " HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()". INLINE COMMENTS src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java:611 It was added by me actually. Because I checked Bytes.toBytes(ByteBuffer bb) and found that it's different from this one. But this one is the same as Bytes.getBytes(ByteBuffer bb). These names are really confusing. REVISION DETAIL https://reviews.facebook.net/D2355 BRANCH getbytes
      Hide
      Hudson added a comment -

      Integrated in HBase-TRUNK-security #146 (See https://builds.apache.org/job/HBase-TRUNK-security/146/)
      HBASE-5591 ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes() (Scott Chen) (Revision 1304047)

      Result = SUCCESS
      tedyu :
      Files :

      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
      Show
      Hudson added a comment - Integrated in HBase-TRUNK-security #146 (See https://builds.apache.org/job/HBase-TRUNK-security/146/ ) HBASE-5591 ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes() (Scott Chen) (Revision 1304047) Result = SUCCESS tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
      Hide
      Phabricator added a comment -

      stack has commented on the revision "HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()".

      INLINE COMMENTS
      src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java:611 Funny. If it was added by you, then I'd say its your prerogative to remove it! Good stuff.

      REVISION DETAIL
      https://reviews.facebook.net/D2355

      BRANCH
      getbytes

      Show
      Phabricator added a comment - stack has commented on the revision " HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()". INLINE COMMENTS src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java:611 Funny. If it was added by you, then I'd say its your prerogative to remove it! Good stuff. REVISION DETAIL https://reviews.facebook.net/D2355 BRANCH getbytes
      Hide
      Phabricator added a comment -

      sc has commented on the revision "HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()".

      INLINE COMMENTS
      src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java:611 Sorry for the trouble. Thanks for bearing with me

      REVISION DETAIL
      https://reviews.facebook.net/D2355

      BRANCH
      getbytes

      Show
      Phabricator added a comment - sc has commented on the revision " HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()". INLINE COMMENTS src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java:611 Sorry for the trouble. Thanks for bearing with me REVISION DETAIL https://reviews.facebook.net/D2355 BRANCH getbytes
      Hide
      Hudson added a comment -

      Integrated in HBase-TRUNK #2693 (See https://builds.apache.org/job/HBase-TRUNK/2693/)
      HBASE-5591 ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes() (Scott Chen) (Revision 1304047)

      Result = SUCCESS
      tedyu :
      Files :

      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java
      • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
      Show
      Hudson added a comment - Integrated in HBase-TRUNK #2693 (See https://builds.apache.org/job/HBase-TRUNK/2693/ ) HBASE-5591 ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes() (Scott Chen) (Revision 1304047) Result = SUCCESS tedyu : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java
      Hide
      Phabricator added a comment -

      sc has closed the revision "HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()".

      REVISION DETAIL
      https://reviews.facebook.net/D2355

      To: tedyu, dhruba, JIRA, sc
      Cc: stack

      Show
      Phabricator added a comment - sc has closed the revision " HBASE-5591 [jira] ThiftServerRunner.HBaseHandler.toBytes() is identical to Bytes.getBytes()". REVISION DETAIL https://reviews.facebook.net/D2355 To: tedyu, dhruba, JIRA, sc Cc: stack
      Hide
      stack added a comment -

      This was committed to trunk a while back but will leave it open until Scott grants Apache permission.

      Scott Chen Would you mind granting permission for this already committed patch else we'll have to remove it from trunk. Thanks boss.

      Show
      stack added a comment - This was committed to trunk a while back but will leave it open until Scott grants Apache permission. Scott Chen Would you mind granting permission for this already committed patch else we'll have to remove it from trunk. Thanks boss.
      Hide
      Hadoop QA added a comment -

      -1 overall. Here are the results of testing the latest attachment
      http://issues.apache.org/jira/secure/attachment/12547256/ASF.LICENSE.GRANTED--HBASE-5591.D2355.1.patch
      against trunk revision .

      +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 new tests are needed for this patch.
      Also please list what manual steps were performed to verify this patch.

      -1 patch. The patch command could not apply the patch.

      Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2978//console

      This message is automatically generated.

      Show
      Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12547256/ASF.LICENSE.GRANTED--HBASE-5591.D2355.1.patch against trunk revision . +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 new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/2978//console This message is automatically generated.
      Hide
      Scott Chen added a comment -

      Yes, permission granted.
      I couldn't find the UI for granting the permission.
      Does this statement in the comment count?

      Show
      Scott Chen added a comment - Yes, permission granted. I couldn't find the UI for granting the permission. Does this statement in the comment count?
      Hide
      stack added a comment -

      Thank you Scott.

      No check box any more. If you don't want to include it, you say so in a comment (Might be easier on you fellas with your auto-posting phabricator).

      Thanks for perm.

      Show
      stack added a comment - Thank you Scott. No check box any more. If you don't want to include it, you say so in a comment (Might be easier on you fellas with your auto-posting phabricator). Thanks for perm.

        People

        • Assignee:
          Scott Chen
          Reporter:
          Scott Chen
        • Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development