You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/08/17 18:40:39 UTC

[GitHub] [hbase] bitoffdev opened a new pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

bitoffdev opened a new pull request #2268:
URL: https://github.com/apache/hbase/pull/2268


   Resolves https://issues.apache.org/jira/browse/HBASE-24874
   
   ## Changelog
   
   - Fix hbase-shell access in JDK 11 for calls to
     TableDescriptorBuilder.toCoprocessorDescriptor and
     ModifiableTableDescriptor.toStringTableAttributes.
   - Allow coprocessors to be specified using a Ruby hash in the hbase-shell alter
     command and replace usage in the help text. The previous String overload of
     the alter command will continue to work and is still covered by a unit test,
     but will no longer be suggested in the alter command help.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] bitoffdev commented on a change in pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
bitoffdev commented on a change in pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#discussion_r472221019



##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -587,7 +588,10 @@ def get_column_families(table_name)
 
     def get_table_attributes(table_name)
       tableExists(table_name)
-      @admin.getDescriptor(TableName.valueOf(table_name)).toStringTableAttributes
+      td = @admin.getDescriptor TableName.valueOf(table_name)
+      method = td.java_class.declared_method :toStringTableAttributes
+      method.accessible = true

Review comment:
       Added a note... I also left a TODO in case someone else comes along with an idea to makes this better.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#issuecomment-675518880


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 27s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 22s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 12s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 50s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 45s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 28s |  hbase-client in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  0s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 49s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 49s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 43s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 26s |  hbase-client in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 11s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  |   7m  4s |  hbase-shell in the patch passed.  |
   |  |   |  33m 12s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2268 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 5c6534e3bb1c 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ea26463a33 |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-client.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-client.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/testReport/ |
   | Max. process+thread count | 2283 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-shell U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] bitoffdev commented on a change in pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
bitoffdev commented on a change in pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#discussion_r472225473



##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -664,6 +668,40 @@ def alter_status(table_name)
       puts 'Done.'
     end
 
+    #----------------------------------------------------------------------------------------------
+    # Use our internal logic to convert from "spec string" format to a coprocessor descriptor
+    #
+    # Provided for backwards shell compatibility
+    #
+    # @param [String] spec_str
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_spec_str(spec_str)

Review comment:
       Done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] saintstack merged pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
saintstack merged pull request #2268:
URL: https://github.com/apache/hbase/pull/2268


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] ndimiduk commented on a change in pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#discussion_r471764312



##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -664,6 +668,40 @@ def alter_status(table_name)
       puts 'Done.'
     end
 
+    #----------------------------------------------------------------------------------------------
+    # Use our internal logic to convert from "spec string" format to a coprocessor descriptor
+    #
+    # Provided for backwards shell compatibility
+    #
+    # @param [String] spec_str
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_spec_str(spec_str)

Review comment:
       If you want to do this, please place a fat warning in the docs over on the `TableDescriptorBuilder.toCoprocessorDescriptor` method.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] bitoffdev commented on a change in pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
bitoffdev commented on a change in pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#discussion_r472220333



##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -664,6 +668,40 @@ def alter_status(table_name)
       puts 'Done.'
     end
 
+    #----------------------------------------------------------------------------------------------
+    # Use our internal logic to convert from "spec string" format to a coprocessor descriptor
+    #
+    # Provided for backwards shell compatibility
+    #
+    # @param [String] spec_str
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_spec_str(spec_str)
+      method = TableDescriptorBuilder.java_class.declared_method_smart :toCoprocessorDescriptor
+      method.accessible = true
+      result = method.invoke(nil, spec_str).to_java
+      # unpack java's Optional to be more rubonic
+      return result.isPresent ? result.get : nil
+    end
+
+    #----------------------------------------------------------------------------------------------
+    # Use CoprocessorDescriptorBuilder to convert a Hash to CoprocessorDescriptor
+    #
+    # @param [Hash] spec column descriptor specification
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_hash(spec)
+      classname = spec['CLASSNAME']
+      jar_path = spec['JAR_PATH']
+      priority = spec['PRIORITY']
+      properties = spec['PROPERTIES']
+
+      builder = CoprocessorDescriptorBuilder.newBuilder classname

Review comment:
       Good call, the current NullPointerException isn't super user friendly. I just updated the patch to handle this more user-friendly.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#issuecomment-675539262


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 12s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 51s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 49s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 41s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 26s |  hbase-client in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m  3s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 50s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 50s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 45s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 25s |  hbase-client in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  9s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  |   7m  4s |  hbase-shell in the patch passed.  |
   |  |   |  32m 37s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2268 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux b8fe12441cd1 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ea26463a33 |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-client.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-client.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/testReport/ |
   | Max. process+thread count | 2306 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-shell U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#issuecomment-675052927


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  2s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   ||| _ Patch Compile Tests _ |
   | -0 :warning: |  rubocop  |   0m 18s |  The patch generated 27 new + 536 unchanged - 5 fixed = 563 total (was 541)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 11s |  The patch does not generate ASF License warnings.  |
   |  |   |   2m 47s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2268 |
   | Optional Tests | dupname asflicense rubocop |
   | uname | Linux b6f3ffecb606 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ea26463a33 |
   | rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/1/artifact/yetus-general-check/output/diff-patch-rubocop.txt |
   | Max. process+thread count | 47 (vs. ulimit of 12500) |
   | modules | C: hbase-shell U: hbase-shell |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) rubocop=0.80.0 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#issuecomment-675538148


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 27s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 19s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 46s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 38s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 35s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 23s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 45s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 45s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 35s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  1s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  |   6m 58s |  hbase-shell in the patch passed.  |
   |  |   |  30m 41s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2268 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux dded57bf35ca 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ea26463a33 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/testReport/ |
   | Max. process+thread count | 2343 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-shell U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#issuecomment-675537428


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 27s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 23s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 39s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 56s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 20s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 37s |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 18s |  The patch generated 31 new + 546 unchanged - 5 fixed = 577 total (was 551)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  10m 57s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   1m  6s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 24s |  The patch does not generate ASF License warnings.  |
   |  |   |  29m 28s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2268 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle rubocop |
   | uname | Linux 3e67e7c2aca1 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ea26463a33 |
   | rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/artifact/yetus-general-check/output/diff-patch-rubocop.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-shell U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 rubocop=0.80.0 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#issuecomment-675059773


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 30s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 19s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  5s |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   6m 49s |  hbase-shell in the patch passed.  |
   |  |   |  17m 20s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2268 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux 9b2dd4aadf25 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ea26463a33 |
   | Default Java | 2020-01-14 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/1/testReport/ |
   | Max. process+thread count | 2379 (vs. ulimit of 12500) |
   | modules | C: hbase-shell U: hbase-shell |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] saintstack commented on a change in pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#discussion_r471914215



##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -664,6 +668,40 @@ def alter_status(table_name)
       puts 'Done.'
     end
 
+    #----------------------------------------------------------------------------------------------
+    # Use our internal logic to convert from "spec string" format to a coprocessor descriptor
+    #
+    # Provided for backwards shell compatibility
+    #
+    # @param [String] spec_str
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_spec_str(spec_str)
+      method = TableDescriptorBuilder.java_class.declared_method_smart :toCoprocessorDescriptor
+      method.accessible = true
+      result = method.invoke(nil, spec_str).to_java
+      # unpack java's Optional to be more rubonic
+      return result.isPresent ? result.get : nil
+    end
+
+    #----------------------------------------------------------------------------------------------
+    # Use CoprocessorDescriptorBuilder to convert a Hash to CoprocessorDescriptor
+    #
+    # @param [Hash] spec column descriptor specification
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_hash(spec)
+      classname = spec['CLASSNAME']
+      jar_path = spec['JAR_PATH']
+      priority = spec['PRIORITY']
+      properties = spec['PROPERTIES']
+
+      builder = CoprocessorDescriptorBuilder.newBuilder classname

Review comment:
       Its ok not checking for nil classname?

##########
File path: hbase-shell/src/main/ruby/shell/commands/alter.rb
##########
@@ -53,19 +53,19 @@ def help
 
   hbase> alter 't1', MAX_FILESIZE => '134217728'
 
-You can add a table coprocessor by setting a table coprocessor attribute:
+You can add a table coprocessor by setting a table coprocessor attribute. Only the CLASSNAME is
+required in the coprocessor specification.
 
-  hbase> alter 't1',
-    'coprocessor'=>'hdfs:///foo.jar|com.foo.FooRegionObserver|1001|arg1=1,arg2=2'
+  hbase> alter 't1', 'coprocessor' => {
+           'CLASSNAME' => 'org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver',
+           'JAR_PATH' => 'hdfs:///foo.jar'
+           'PRIORITY' => 12,
+           'PROPERTIES' => {'a' => '17' }

Review comment:
       nice

##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -587,7 +588,10 @@ def get_column_families(table_name)
 
     def get_table_attributes(table_name)
       tableExists(table_name)
-      @admin.getDescriptor(TableName.valueOf(table_name)).toStringTableAttributes
+      td = @admin.getDescriptor TableName.valueOf(table_name)
+      method = td.java_class.declared_method :toStringTableAttributes
+      method.accessible = true

Review comment:
       Do you want to say in comment why you are doing this access setting?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#issuecomment-675517953


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 22s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 22s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 37s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   0m 41s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   0m 59s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 28s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 38s |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 19s |  The patch generated 31 new + 546 unchanged - 5 fixed = 577 total (was 551)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m 16s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   1m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 26s |  The patch does not generate ASF License warnings.  |
   |  |   |  31m 35s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2268 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle rubocop |
   | uname | Linux f78cb7a734ae 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ea26463a33 |
   | rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/artifact/yetus-general-check/output/diff-patch-rubocop.txt |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-shell U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 rubocop=0.80.0 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#issuecomment-675059579


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 36s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  3s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 16s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 40s |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 13s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   6m 59s |  hbase-shell in the patch passed.  |
   |  |   |  16m 54s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2268 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux e9e58cff7aaa 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ea26463a33 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/1/testReport/ |
   | Max. process+thread count | 2343 (vs. ulimit of 12500) |
   | modules | C: hbase-shell U: hbase-shell |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Apache-HBase commented on pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#issuecomment-675517578


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 27s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 28s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 20s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 44s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 33s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 16s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 30s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 46s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 46s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 35s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m  0s |  hbase-client in the patch passed.  |
   | +1 :green_heart: |  unit  |   6m 59s |  hbase-shell in the patch passed.  |
   |  |   |  30m 57s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2268 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 495d543a517f 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / ea26463a33 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/testReport/ |
   | Max. process+thread count | 2325 (vs. ulimit of 12500) |
   | modules | C: hbase-client hbase-shell U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2268/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] bitoffdev commented on a change in pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
bitoffdev commented on a change in pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#discussion_r472221320



##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -664,6 +668,40 @@ def alter_status(table_name)
       puts 'Done.'
     end
 
+    #----------------------------------------------------------------------------------------------
+    # Use our internal logic to convert from "spec string" format to a coprocessor descriptor
+    #
+    # Provided for backwards shell compatibility
+    #
+    # @param [String] spec_str
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_spec_str(spec_str)
+      method = TableDescriptorBuilder.java_class.declared_method_smart :toCoprocessorDescriptor
+      method.accessible = true
+      result = method.invoke(nil, spec_str).to_java
+      # unpack java's Optional to be more rubonic
+      return result.isPresent ? result.get : nil
+    end
+
+    #----------------------------------------------------------------------------------------------
+    # Use CoprocessorDescriptorBuilder to convert a Hash to CoprocessorDescriptor
+    #
+    # @param [Hash] spec column descriptor specification
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_hash(spec)
+      classname = spec['CLASSNAME']

Review comment:
       Makes sense, done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] ndimiduk commented on a change in pull request #2268: HBASE-24874 Fix hbase-shell access to ModifiableTableDescriptor methods

Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2268:
URL: https://github.com/apache/hbase/pull/2268#discussion_r471764673



##########
File path: hbase-shell/src/main/ruby/hbase/admin.rb
##########
@@ -664,6 +668,40 @@ def alter_status(table_name)
       puts 'Done.'
     end
 
+    #----------------------------------------------------------------------------------------------
+    # Use our internal logic to convert from "spec string" format to a coprocessor descriptor
+    #
+    # Provided for backwards shell compatibility
+    #
+    # @param [String] spec_str
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_spec_str(spec_str)
+      method = TableDescriptorBuilder.java_class.declared_method_smart :toCoprocessorDescriptor
+      method.accessible = true
+      result = method.invoke(nil, spec_str).to_java
+      # unpack java's Optional to be more rubonic
+      return result.isPresent ? result.get : nil
+    end
+
+    #----------------------------------------------------------------------------------------------
+    # Use CoprocessorDescriptorBuilder to convert a Hash to CoprocessorDescriptor
+    #
+    # @param [Hash] spec column descriptor specification
+    # @return [ColumnDescriptor]
+    def coprocessor_descriptor_from_hash(spec)
+      classname = spec['CLASSNAME']

Review comment:
       should these `'CLASSPATH'`, `'JAR_PATH'`, &c. be defined from `hbase_constants.rb` ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org