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/10 14:57:09 UTC

[GitHub] [hbase] bitoffdev opened a new pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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


   Resolves https://issues.apache.org/jira/browse/HBASE-24806
   
   ## High-Level Overview
   
   1. Create exception handler for shell (to hide tracebacks unless debug is on). The handler just replaces exceptions with SystemExit.
   2. Get rid of a recently introduced IRB warning (`can't alias help from irb_help`).
   3. Create a new module Hbase::Loader that can find ruby scripts using JRuby's loader. This is better than IRB::Loader because it can find anything that can be found directly with `load`, including files inside a Jar. It is necessary so that our shell can _evaluate_ scripts in the $LOAD_PATH/classpath.
   4. Update the test runner to catch SystemExits. This will ensure that our unit tests stay potent and cannot fail unnoticed.
   
   ## Changelog
   
   - Move exception handler from Shell::Shell#eval_io to new method,
     Shell::Shell#exception_handler
   - Add unit tests for Shell::Shell#exception_handler
   - Change Shell::Shell#eval_io to no longer raise SystemExit when any error is
     seen and update unit test
   - Update ruby test runner to catch SystemExit and fail to avoid tests that
     cause the test runner to incorrectly exit successfully
   - Add Hbase::Loader module to find ruby scripts in the $LOAD_PATH and classpath
     using JRuby's loader.
   - In hbase-shell, install IRB commands before exporting HBase commands. The
     HBase commands will override the IRB commands, and no warning will be
     printed.
   


----------------------------------------------------------------
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 pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

Posted by GitBox <gi...@apache.org>.
ndimiduk commented on pull request #2232:
URL: https://github.com/apache/hbase/pull/2232#issuecomment-672345272


   There's also,
   
   ```
   ===============================================================================
   Error: test_describe_should_return_a_description_and_quotas(Hbase::AdminMethodsTest):
     NoMethodError: undefined method `toStringTableAttributes' for #<Java::OrgApacheHadoopHbaseClient::TableDescriptorBuilder::ModifyableTableDescriptor:0x421757ca>
     Did you mean?  toStringCustomizedValues
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/hbase/admin.rb:590:in `get_table_attributes'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell/commands/describe.rb:40:in `command'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell/commands.rb:49:in `block in command_safe'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell/commands.rb:122:in `translate_hbase_exceptions'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell/commands.rb:49:in `command_safe'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell.rb:199:in `internal_command'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell.rb:191:in `command'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/test/ruby/test_helper.rb:55:in `command'
   src/test/ruby/hbase/admin_test.rb:528:in `block in test_describe_should_return_a_description_and_quotas'
        525:               LIMIT => '1G',
        526:               POLICY => NO_INSERTS,
        527:               TABLE => @create_test_name)
     => 528:       output = capture_stdout { command(:describe, @create_test_name) }
        529: 
        530:       assert(output.include?("Table #{@create_test_name} is ENABLED"))
        531:       assert(output.include?('COLUMN FAMILIES DESCRIPTION'))
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/test/ruby/test_helper.rb:161:in `capture_stdout'
   src/test/ruby/hbase/admin_test.rb:528:in `block in test_describe_should_return_a_description_and_quotas'
   ===============================================================================
   ```


----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -320,10 +323,20 @@ def eval_io(io)
       scanner = RubyLex.new
       scanner.set_input(io)
 
+      scanner.each_top_level_statement do |statement, linenum|
+        puts(workspace.evaluate(nil, statement, filename, linenum))

Review comment:
       Yup! Credit to @busbey for figuring this out in HBASE-11658 (this code used to exist in `bin/hirb.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



[GitHub] [hbase] ndimiduk commented on pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

Posted by GitBox <gi...@apache.org>.
ndimiduk commented on pull request #2232:
URL: https://github.com/apache/hbase/pull/2232#issuecomment-672343963


   @bitoffdev do you know how to find the test log files? I'm looking at PR-2232/1/artifact/yetus-jdk11-hadoop3-check/output, there's a file called test_logs.zip in the Jenkins build archive. In that archive, I find the `TEST-...TestClass.xml` file for the failed test and look for more info. For example, in `TEST...-TestAdminShell.xml`, I find
   
   ```
   ===============================================================================
   Error: test_alter_should_be_able_to_change_coprocessor_attributes(Hbase::AdminAlterTableTest): NoMethodError: undefined method `setCoprocessorWithSpec' for #<Java::OrgApacheHadoopHbaseClient::TableDescriptorBuilder::ModifyableTableDescriptor:0xb6bc6ac>
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/hbase/admin.rb:781:in `block in alter'
   org/jruby/RubyHash.java:1350:in `each'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/hbase/admin.rb:771:in `block in alter'
   org/jruby/RubyArray.java:1735:in `each'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/hbase/admin.rb:687:in `alter'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell/commands/alter.rb:103:in `command'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell/commands.rb:49:in `block in command_safe'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell/commands.rb:122:in `translate_hbase_exceptions'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell/commands.rb:49:in `command_safe'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell.rb:199:in `internal_command'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/main/ruby/shell.rb:191:in `command'
   /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-2232/yetus-jdk11-hadoop3-check/src/hbase-shell/src/test/ruby/test_helper.rb:55:in `command'
   src/test/ruby/hbase/admin_test.rb:958:in `block in test_alter_should_be_able_to_change_coprocessor_attributes'
        955:       # eval() is used to convert a string to regex
        956:       assert_no_match(eval("/" + class_name + "/"), admin.describe(@test_name))
        957:       assert_no_match(eval("/" + cp_key + "/"), admin.describe(@test_name))
     => 958:       command(:alter, @test_name, 'METHOD' => 'table_att', cp_key => cp_value)
        959:       assert_match(eval("/" + class_name + "/"), admin.describe(@test_name))
        960:       assert_match(eval("/" + cp_key + "\\$(\\d+)/"), admin.describe(@test_name))
        961:     end
   ===============================================================================
   ```


----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 46s |  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 25s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   5m 41s |  master passed  |
   | -0 :warning: |  javadoc  |   0m 19s |  root in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 15s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   5m 41s |  the patch passed  |
   | -0 :warning: |  javadoc  |   0m 18s |  root in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 171m 20s |  root in the patch failed.  |
   |  |   | 188m 44s |   |
   
   
   | 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-2232/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2232 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux 1ae0548b9111 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 / c1c2e160ec |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-root.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-root.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/2/testReport/ |
   | Max. process+thread count | 6646 (vs. ulimit of 12500) |
   | modules | C: hbase-shell . U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/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] Apache-HBase commented on pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   5m 51s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 28s |  Maven dependency ordering for branch  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m  8s |  Maven dependency ordering for patch  |
   | -0 :warning: |  rubocop  |   0m 16s |  The patch generated 8 new + 85 unchanged - 7 fixed = 93 total (was 92)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 24s |  The patch does not generate ASF License warnings.  |
   |  |   |   8m 38s |   |
   
   
   | 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-2232/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2232 |
   | Optional Tests | dupname asflicense rubocop |
   | uname | Linux d48a93c503bf 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / c1c2e160ec |
   | rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/2/artifact/yetus-general-check/output/diff-patch-rubocop.txt |
   | Max. process+thread count | 52 (vs. ulimit of 12500) |
   | modules | C: . hbase-shell U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/2/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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 52s |  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 _ |
   | +0 :ok: |  mvndep  |   0m 25s |  Maven dependency ordering for branch  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m  7s |  Maven dependency ordering for patch  |
   | -0 :warning: |  rubocop  |   0m 18s |  The patch generated 10 new + 85 unchanged - 7 fixed = 95 total (was 92)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 24s |  The patch does not generate ASF License warnings.  |
   |  |   |   3m 31s |   |
   
   
   | 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-2232/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2232 |
   | Optional Tests | dupname asflicense rubocop |
   | uname | Linux 5758d696fa75 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 / 7b099eaa75 |
   | rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/1/artifact/yetus-general-check/output/diff-patch-rubocop.txt |
   | Max. process+thread count | 52 (vs. ulimit of 12500) |
   | modules | C: . hbase-shell U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/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] bitoffdev commented on pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

Posted by GitBox <gi...@apache.org>.
bitoffdev commented on pull request #2232:
URL: https://github.com/apache/hbase/pull/2232#issuecomment-675474620


   Also, the failures mentioned by @ndimiduk are described and fixed by https://issues.apache.org/jira/browse/HBASE-24874. They are unrelated to this patch.


----------------------------------------------------------------
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 pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

Posted by GitBox <gi...@apache.org>.
bitoffdev commented on pull request #2232:
URL: https://github.com/apache/hbase/pull/2232#issuecomment-675473801


   > I like the idea of removing exception spew. Maybe shove a before and after up in the JIRA for folks to see?
   
   The end-user functionality hasn't changed. This functionality used to exist in `bin/hirb.rb`. Moving it here makes it easier to test and reuse.


----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 48s |  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 10s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   5m 11s |  master passed  |
   | -0 :warning: |  javadoc  |   0m 19s |  root in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 47s |  the patch passed  |
   | -0 :warning: |  javadoc  |   0m 17s |  root in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 188m  3s |  root in the patch failed.  |
   |  |   | 203m 15s |   |
   
   
   | 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-2232/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2232 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux 22acdedd77c5 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 / 7b099eaa75 |
   | Default Java | 2020-01-14 |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-root.txt |
   | javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-root.txt |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/1/testReport/ |
   | Max. process+thread count | 6061 (vs. ulimit of 12500) |
   | modules | C: hbase-shell . U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/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] ndimiduk commented on a change in pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -335,12 +348,9 @@ def eval_io(io)
         # Include the 'ERROR' string to try to make transition easier for scripts that
         # may have already been relying on grepping output.
         puts "ERROR #{e.class}: #{message}"
-        if $fullBacktrace
-          # re-raising the will include a backtrace and exit.
-          raise e
-        else
-          exit 1
-        end
+        raise e unless hide_traceback
+
+        exit 1

Review comment:
       Control flow returns to this line after a `raise` ? `raise` is an expression in this language..?

##########
File path: hbase-shell/src/test/ruby/shell/shell_test.rb
##########
@@ -123,14 +124,35 @@ class FooC; end
 
     # check that at least one of the HBase constants is present while evaluating
     io = StringIO.new <<~EOF
-      ROWPREFIXFILTER
+      ROWPREFIXFILTER.dump
     EOF
     output = capture_stdout { @shell.eval_io(io) }
     assert_match(/"ROWPREFIXFILTER"/, output)
   end
 
   #-----------------------------------------------------------------------------
 
+  define_test 'Shell::Shell#exception_handler should hide traceback' do
+    class TestException < RuntimeError; end
+    hide_traceback = true

Review comment:
       why do you both define a variable `hide_traceback = true` and also pass `true` to the `exception_handler` -- it's redundant?

##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -320,10 +323,20 @@ def eval_io(io)
       scanner = RubyLex.new
       scanner.set_input(io)
 
+      scanner.each_top_level_statement do |statement, linenum|
+        puts(workspace.evaluate(nil, statement, filename, linenum))

Review comment:
       this propagates source files and line numbers for the purpose of error handling? nice!




----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 19s |  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 23s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   3m 38s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m 21s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 13s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   3m 23s |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   2m 21s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 267m  6s |  root in the patch failed.  |
   |  |   | 282m 55s |   |
   
   
   | 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-2232/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2232 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux 3620ce9b3bbb 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 / c1c2e160ec |
   | Default Java | 1.8.0_232 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/2/testReport/ |
   | Max. process+thread count | 3736 (vs. ulimit of 12500) |
   | modules | C: hbase-shell . U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -335,12 +348,9 @@ def eval_io(io)
         # Include the 'ERROR' string to try to make transition easier for scripts that
         # may have already been relying on grepping output.
         puts "ERROR #{e.class}: #{message}"
-        if $fullBacktrace
-          # re-raising the will include a backtrace and exit.
-          raise e
-        else
-          exit 1
-        end
+        raise e unless hide_traceback
+
+        exit 1

Review comment:
       I also added unit tests that cover/verify this.




----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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


   


----------------------------------------------------------------
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 pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

Posted by GitBox <gi...@apache.org>.
bitoffdev commented on pull request #2232:
URL: https://github.com/apache/hbase/pull/2232#issuecomment-671985958


   Seems like none of the unit test failures are caused by this patch.
   
   - Unit failed for [Build #1 and #2, JDK 11 Hadoop 3](https://github.com/apache/hbase/pull/2232#issuecomment-671542484) because a test in hbase-shell was already defined.
     - I'm seeing this same failure for other currently-open PRs: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2236/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-shell.txt
     - I think this could possibly be caused by our use of `System.setProperty` when running shell tests. I'll open a Jira for this.
   - Unit failed for [Builds #1 and #2, JDK 8 Hadoop 3](https://github.com/apache/hbase/pull/2232#issuecomment-671582255) because of hbase-server, which was not modified in this PR.


----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  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 25s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |   4m 34s |  master passed  |
   | +1 :green_heart: |  javadoc  |   2m 48s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 14s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   4m 46s |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   3m  7s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  | 271m 40s |  root in the patch failed.  |
   |  |   | 290m 49s |   |
   
   
   | 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-2232/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/2232 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux 524ac72e609e 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 / 7b099eaa75 |
   | Default Java | 1.8.0_232 |
   | unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/1/testReport/ |
   | Max. process+thread count | 3891 (vs. ulimit of 12500) |
   | modules | C: hbase-shell . U: . |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2232/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 pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

Posted by GitBox <gi...@apache.org>.
saintstack commented on pull request #2232:
URL: https://github.com/apache/hbase/pull/2232#issuecomment-675241828


   @bitoffdev You see Nick's comments above boss?


----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -320,10 +323,20 @@ def eval_io(io)
       scanner = RubyLex.new
       scanner.set_input(io)
 
+      scanner.each_top_level_statement do |statement, linenum|
+        puts(workspace.evaluate(nil, statement, filename, linenum))
+      end
+      nil
+    end
+
+    ##
+    # Runs a block and logs exception from both Ruby and Java, optionally discarding the traceback
+    #
+    # @param [Boolean] hide_traceback if true, Exceptions will be converted to
+    #   a SystemExit so that the traceback is not printed
+    def self.exception_handler(hide_traceback)
       begin
-        scanner.each_top_level_statement do |statement, linenum|
-          puts(workspace.evaluate(nil, statement, 'stdin', linenum))
-        end
+        yield

Review comment:
       The `yield` ensures that whatever block is passed to `exception_handler` is executed within our begin/rescue, which is how we are able to catch and translate exceptions to a SystemExit to avoid printing the full traceback.




----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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



##########
File path: hbase-shell/src/test/ruby/shell/shell_test.rb
##########
@@ -123,14 +124,35 @@ class FooC; end
 
     # check that at least one of the HBase constants is present while evaluating
     io = StringIO.new <<~EOF
-      ROWPREFIXFILTER
+      ROWPREFIXFILTER.dump
     EOF
     output = capture_stdout { @shell.eval_io(io) }
     assert_match(/"ROWPREFIXFILTER"/, output)
   end
 
   #-----------------------------------------------------------------------------
 
+  define_test 'Shell::Shell#exception_handler should hide traceback' do
+    class TestException < RuntimeError; end
+    hide_traceback = true

Review comment:
       Agreed. I removed the unused variables.




----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -335,12 +348,9 @@ def eval_io(io)
         # Include the 'ERROR' string to try to make transition easier for scripts that
         # may have already been relying on grepping output.
         puts "ERROR #{e.class}: #{message}"
-        if $fullBacktrace
-          # re-raising the will include a backtrace and exit.
-          raise e
-        else
-          exit 1
-        end
+        raise e unless hide_traceback
+
+        exit 1

Review comment:
       Ruby allows one-line conditionals such as `<statement> if <condition>`, which is equivalent to `if <condition>; <statement>; end`. It's a fun feature of Ruby :smile:.
   
   So, this logic will raise SystemExit (via `exit 1`) if we want to hide the traceback.




----------------------------------------------------------------
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 pull request #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

Posted by GitBox <gi...@apache.org>.
bitoffdev commented on pull request #2232:
URL: https://github.com/apache/hbase/pull/2232#issuecomment-671987465


   If @busbey or @madrob get a chance, I'd appreciate a review. Cheers!


----------------------------------------------------------------
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 #2232: HBASE-24806 Small Updates to Functionality of Shell IRB Workspace

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -320,10 +323,20 @@ def eval_io(io)
       scanner = RubyLex.new
       scanner.set_input(io)
 
+      scanner.each_top_level_statement do |statement, linenum|
+        puts(workspace.evaluate(nil, statement, filename, linenum))
+      end
+      nil
+    end
+
+    ##
+    # Runs a block and logs exception from both Ruby and Java, optionally discarding the traceback
+    #
+    # @param [Boolean] hide_traceback if true, Exceptions will be converted to
+    #   a SystemExit so that the traceback is not printed
+    def self.exception_handler(hide_traceback)
       begin
-        scanner.each_top_level_statement do |statement, linenum|
-          puts(workspace.evaluate(nil, statement, 'stdin', linenum))
-        end
+        yield

Review comment:
       We need this begin/rescue w/ a yield in the middle of it?

##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -320,10 +323,20 @@ def eval_io(io)
       scanner = RubyLex.new
       scanner.set_input(io)
 
+      scanner.each_top_level_statement do |statement, linenum|
+        puts(workspace.evaluate(nil, statement, filename, linenum))
+      end
+      nil
+    end
+
+    ##
+    # Runs a block and logs exception from both Ruby and Java, optionally discarding the traceback
+    #
+    # @param [Boolean] hide_traceback if true, Exceptions will be converted to
+    #   a SystemExit so that the traceback is not printed
+    def self.exception_handler(hide_traceback)
       begin
-        scanner.each_top_level_statement do |statement, linenum|
-          puts(workspace.evaluate(nil, statement, 'stdin', linenum))
-        end
+        yield

Review comment:
       Whats going on here? We are skipping output of the exception stack trace?




----------------------------------------------------------------
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