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 2021/11/30 23:01:18 UTC

[GitHub] [hbase] busbey opened a new pull request #3901: WIP use IRB sessions instead of trying to make it ourselves

busbey opened a new pull request #3901:
URL: https://github.com/apache/hbase/pull/3901


   copying the problem summary from HBASE-26469
   
   > The `clean_exit.rb` invocations ought to exit with success, exit code 0.
   > 
   > |        | 1.4.14 | 1.7.1 | 2.0.6 | 2.1.9 | 2.2.7 | 2.3.7 | 2.4.8 | master |
   > | --- |-------|-----|------|-------|------|------|------|--------|
   > | cli, default |    0 |    0   |    0   |    0   |    0   |    0   |    1   |    1*   |
   > | cli, -n |         0 |    0   |    0   |    0   |    0   |    0   |    1   |  hang   |
   > | stdin, default |  0 |    0   |    0   |    0   |    0   |    0   |    0   |    0    |
   > | stdin, -n |       1 |    1   |    1   |    1   |    1   |    1   |    1*  |    1*   |
   > 
   > The `error_exit.rb` invocation should return a non-zero exit code, unless we're specifically trying to match a normal hbase shell session.
   > 
   > |     |     1.4.14 | 1.7.1 | 2.0.6 | 2.1.9 | 2.2.7 | 2.3.7 | 2.4.8 | master |
   > |-----|----|-----|-----|------|----| ----| ---- | ----|
   > | cli, default |   1 |    1   |    1   |    1   |    1   |    1   |    1*  |    1*   |
   > | cli, -n |        1 |    1   |    1   |    1   |    1   |    1   |    1*  |  hang   |
   > | stdin, default | 0 |    0   |    0   |    0   |    0   |    0   |    0   |    0    |
   > | stdin, -n |      1 |    1   |    1   |    1   |    1   |    1   |    1*  |    1*   |
   > 
   > In cases marked with * the error details are different.
   
   this WIP PR on branch-2.4 has the following summary:
   
   clean_exit:
   
   |  | PR |
   |--|---|
   | cli, default |    0 |
   | cli, -n |         0 | 
   | stdin, default |  0 |
   | stdin, -n |       0 |
   
   error_exit:
   
   |  | PR |
   |--|---|
   | cli, default |    0 |
   | cli, -n |         1 | 
   | stdin, default |  0 |
   | stdin, -n |       0 |
   
   none of the error_exit give unrelated error messages, e.g
   
   ```
   (base) sbusbey@Seans-MacBook-Pro hbase-2.4.9-SNAPSHOT % ./bin/hbase shell -n ../tmp/error_exit.rb
   2021-11-30 13:30:21,656 WARN  [main] util.NativeCodeLoader: Unable to load native-hadoop library for your platform... using builtin-java classes where applicable
   hbase:001:0> list
   TABLE
   0 row(s)
   Took 0.3448 seconds
   => []
   hbase:002:0> exit 1
   (base) sbusbey@Seans-MacBook-Pro hbase-2.4.9-SNAPSHOT % echo $?
   1
   
   ```
   
   full log attached: [hbase-2.4.9-SNAP-with-fix-exit-behavior.log](https://github.com/apache/hbase/files/7629381/hbase-2.4.9-SNAP-with-fix-exit-behavior.log)


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] busbey commented on a change in pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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



##########
File path: hbase-shell/src/main/ruby/jar-bootstrap.rb
##########
@@ -183,61 +183,47 @@ def debug?
 # instance variables (@hbase and @shell) onto Ruby's top-level receiver object known as "main".
 @shell.export_all(self) if top_level_definitions
 
+require 'irb'
+require 'irb/ext/change-ws'
+require 'irb/hirb'
+
+# Configure IRB
+IRB.setup(nil)
+IRB.conf[:PROMPT][:CUSTOM] = {
+  :PROMPT_I => "%N:%03n:%i> ",
+  :PROMPT_S => "%N:%03n:%i%l ",
+  :PROMPT_C => "%N:%03n:%i* ",
+  :RETURN => "=> %s\n"
+}
+
+IRB.conf[:IRB_NAME] = 'hbase'
+IRB.conf[:AP_NAME] = 'hbase'
+IRB.conf[:PROMPT_MODE] = :CUSTOM
+IRB.conf[:BACK_TRACE_LIMIT] = 0 unless $fullBackTrace
+
+# Create a workspace we'll use across sessions.
+workspace = @shell.get_workspace
+
 # If script2run, try running it.  If we're in interactive mode, will go on to run the shell unless
 # script calls 'exit' or 'exit 0' or 'exit errcode'.
-require 'shell/hbase_loader'
 if script2run
-  ::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(Hbase::Loader.file_for_load(script2run), filename = script2run) }
+  ::Shell::Shell.exception_handler(!$fullBackTrace) do
+    IRB::HIRB.new(workspace, IRB::HBaseLoader.file_for_load(script2run)).run
+  end
+  unless @shell.exit_code.nil?
+    if interactive
+      exit
+    else
+      exit @shell.exit_code
+    end
+  end
 end
 
-# If we are not running interactively, evaluate standard input
-::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(STDIN) } unless interactive
-
 if interactive
   # Output a banner message that tells users where to go for help
   @shell.print_banner
-
-  require 'irb'
-  require 'irb/ext/change-ws'
-  require 'irb/hirb'
-
-  module IRB
-    # Override of the default IRB.start
-    def self.start(ap_path = nil)
-      $0 = File.basename(ap_path, '.rb') if ap_path
-
-      IRB.setup(ap_path)
-      IRB.conf[:PROMPT][:CUSTOM] = {
-        :PROMPT_I => "%N:%03n:%i> ",
-        :PROMPT_S => "%N:%03n:%i%l ",
-        :PROMPT_C => "%N:%03n:%i* ",
-        :RETURN => "=> %s\n"
-      }
-
-      @CONF[:IRB_NAME] = 'hbase'
-      @CONF[:AP_NAME] = 'hbase'
-      @CONF[:PROMPT_MODE] = :CUSTOM
-      @CONF[:BACK_TRACE_LIMIT] = 0 unless $fullBackTrace
-
-      hirb = if @CONF[:SCRIPT]
-               HIRB.new(nil, @CONF[:SCRIPT])
-             else
-               HIRB.new
-             end
-
-      shl = TOPLEVEL_BINDING.receiver.instance_variable_get :'@shell'
-      hirb.context.change_workspace shl.get_workspace
-
-      @CONF[:IRB_RC].call(hirb.context) if @CONF[:IRB_RC]
-      # Storing our current HBase IRB Context as the main context is imperative for several reasons,
-      # including auto-completion.
-      @CONF[:MAIN_CONTEXT] = hirb.context
-
-      catch(:IRB_EXIT) do
-        hirb.eval_input
-      end
-    end
-  end
-
-  IRB.start
+end
+IRB::HIRB.new(workspace).run
+unless interactive or @shell.exit_code.nil?

Review comment:
       yes it is nil so long as nothing in the shell expressly calls `exit`




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] busbey commented on pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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


   exit codes as of this latest patch now look correct to me. the `--non-interactive` flag now gets us a non-zero for the case where we call exit with a non-zero.
   
   
   clean_exit:
   
   |  | PR |
   |--|---|
   | cli, default |    0 |
   | cli, -n |         0 | 
   | stdin, default |  0 |
   | stdin, -n |       0 |
   
   error_exit:
   
   |  | PR |
   |--|---|
   | cli, default |    0 |
   | cli, -n |         1 | 
   | stdin, default |  0 |
   | stdin, -n |       1 |
   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] busbey commented on a change in pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -302,33 +315,13 @@ def get_workspace
       # Install all the hbase commands, constants, and instance variables @shell and @hbase. This
       # will override names that conflict with IRB methods like "help".
       export_all(hbase_receiver)
+      # make it so calling exit will hit our pass-through rather than going directly to IRB
+      hbase_receiver.send :define_singleton_method, :exit, lambda { |rc = 0|

Review comment:
       the call on line 314 to `IRB::ExtendCommandBundle` would overwrite an `exit` method defined on the `HBaseReceiver` class.
   
   we could complicate our lives by passing a different override option to the `extend_object` call that said we didn't want to override methods already defined on the receiver, but then IRB would always print a warning that looked like this below because it also defines an exit method.
   
   ```
       print "irb: warn: can't alias #{to} from #{from}.\n"
   ```




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  6s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.4 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 20s |  branch-2.4 passed  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  branch-2.4 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 25s |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   7m 59s |  hbase-shell in the patch passed.  |
   |  |   |  16m 51s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/2/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3901 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux e37088f14a27 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 | branch-2.4 / 02fa0903a2 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/2/testReport/ |
   | Max. process+thread count | 2591 (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-3901/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] busbey commented on a change in pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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



##########
File path: hbase-shell/src/main/ruby/jar-bootstrap.rb
##########
@@ -183,61 +183,47 @@ def debug?
 # instance variables (@hbase and @shell) onto Ruby's top-level receiver object known as "main".
 @shell.export_all(self) if top_level_definitions
 
+require 'irb'
+require 'irb/ext/change-ws'
+require 'irb/hirb'
+
+# Configure IRB
+IRB.setup(nil)
+IRB.conf[:PROMPT][:CUSTOM] = {
+  :PROMPT_I => "%N:%03n:%i> ",
+  :PROMPT_S => "%N:%03n:%i%l ",
+  :PROMPT_C => "%N:%03n:%i* ",
+  :RETURN => "=> %s\n"
+}
+
+IRB.conf[:IRB_NAME] = 'hbase'
+IRB.conf[:AP_NAME] = 'hbase'
+IRB.conf[:PROMPT_MODE] = :CUSTOM
+IRB.conf[:BACK_TRACE_LIMIT] = 0 unless $fullBackTrace
+
+# Create a workspace we'll use across sessions.
+workspace = @shell.get_workspace
+
 # If script2run, try running it.  If we're in interactive mode, will go on to run the shell unless
 # script calls 'exit' or 'exit 0' or 'exit errcode'.
-require 'shell/hbase_loader'
 if script2run
-  ::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(Hbase::Loader.file_for_load(script2run), filename = script2run) }
+  ::Shell::Shell.exception_handler(!$fullBackTrace) do
+    IRB::HIRB.new(workspace, IRB::HBaseLoader.file_for_load(script2run)).run
+  end
+  unless @shell.exit_code.nil?
+    if interactive
+      exit
+    else
+      exit @shell.exit_code
+    end
+  end
 end
 
-# If we are not running interactively, evaluate standard input
-::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(STDIN) } unless interactive
-
 if interactive
   # Output a banner message that tells users where to go for help
   @shell.print_banner
-
-  require 'irb'
-  require 'irb/ext/change-ws'
-  require 'irb/hirb'
-
-  module IRB
-    # Override of the default IRB.start
-    def self.start(ap_path = nil)
-      $0 = File.basename(ap_path, '.rb') if ap_path
-
-      IRB.setup(ap_path)
-      IRB.conf[:PROMPT][:CUSTOM] = {
-        :PROMPT_I => "%N:%03n:%i> ",
-        :PROMPT_S => "%N:%03n:%i%l ",
-        :PROMPT_C => "%N:%03n:%i* ",
-        :RETURN => "=> %s\n"
-      }
-
-      @CONF[:IRB_NAME] = 'hbase'
-      @CONF[:AP_NAME] = 'hbase'
-      @CONF[:PROMPT_MODE] = :CUSTOM
-      @CONF[:BACK_TRACE_LIMIT] = 0 unless $fullBackTrace
-
-      hirb = if @CONF[:SCRIPT]
-               HIRB.new(nil, @CONF[:SCRIPT])
-             else
-               HIRB.new
-             end
-
-      shl = TOPLEVEL_BINDING.receiver.instance_variable_get :'@shell'
-      hirb.context.change_workspace shl.get_workspace
-
-      @CONF[:IRB_RC].call(hirb.context) if @CONF[:IRB_RC]
-      # Storing our current HBase IRB Context as the main context is imperative for several reasons,
-      # including auto-completion.
-      @CONF[:MAIN_CONTEXT] = hirb.context
-
-      catch(:IRB_EXIT) do
-        hirb.eval_input
-      end
-    end
-  end
-
-  IRB.start
+end
+IRB::HIRB.new(workspace).run
+unless interactive or @shell.exit_code.nil?

Review comment:
       yes it is nil so long as nothing in the shell expressly calls `exit`




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m  9s |  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.  |
   ||| _ branch-2.4 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 55s |  branch-2.4 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 29s |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 14s |  The patch generated 16 new + 57 unchanged - 15 fixed = 73 total (was 72)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  10m 31s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3901 |
   | Optional Tests | dupname asflicense javac rubocop |
   | uname | Linux c04e840c55e2 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 | branch-2.4 / 02fa0903a2 |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/2/artifact/yetus-general-check/output/diff-patch-rubocop.txt |
   | Max. process+thread count | 79 (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-3901/2/console |
   | versions | git=2.17.1 maven=3.6.3 rubocop=0.80.0 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] busbey commented on a change in pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -302,33 +315,13 @@ def get_workspace
       # Install all the hbase commands, constants, and instance variables @shell and @hbase. This
       # will override names that conflict with IRB methods like "help".
       export_all(hbase_receiver)
+      # make it so calling exit will hit our pass-through rather than going directly to IRB
+      hbase_receiver.send :define_singleton_method, :exit, lambda { |rc = 0|

Review comment:
       we don't have a handle to `@shell`, that's true. but if we didn't have the override issue we could e.g. keep the return code state in the receiver and pull it out when the shell needs to return it.




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] busbey closed pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

Posted by GitBox <gi...@apache.org>.
busbey closed pull request #3901:
URL: https://github.com/apache/hbase/pull/3901


   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] joshelser commented on a change in pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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



##########
File path: hbase-shell/src/main/ruby/jar-bootstrap.rb
##########
@@ -183,61 +183,47 @@ def debug?
 # instance variables (@hbase and @shell) onto Ruby's top-level receiver object known as "main".
 @shell.export_all(self) if top_level_definitions
 
+require 'irb'
+require 'irb/ext/change-ws'
+require 'irb/hirb'
+
+# Configure IRB
+IRB.setup(nil)
+IRB.conf[:PROMPT][:CUSTOM] = {
+  :PROMPT_I => "%N:%03n:%i> ",
+  :PROMPT_S => "%N:%03n:%i%l ",
+  :PROMPT_C => "%N:%03n:%i* ",
+  :RETURN => "=> %s\n"
+}
+
+IRB.conf[:IRB_NAME] = 'hbase'
+IRB.conf[:AP_NAME] = 'hbase'
+IRB.conf[:PROMPT_MODE] = :CUSTOM
+IRB.conf[:BACK_TRACE_LIMIT] = 0 unless $fullBackTrace
+
+# Create a workspace we'll use across sessions.
+workspace = @shell.get_workspace
+
 # If script2run, try running it.  If we're in interactive mode, will go on to run the shell unless
 # script calls 'exit' or 'exit 0' or 'exit errcode'.
-require 'shell/hbase_loader'
 if script2run
-  ::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(Hbase::Loader.file_for_load(script2run), filename = script2run) }
+  ::Shell::Shell.exception_handler(!$fullBackTrace) do
+    IRB::HIRB.new(workspace, IRB::HBaseLoader.file_for_load(script2run)).run
+  end
+  unless @shell.exit_code.nil?
+    if interactive
+      exit
+    else
+      exit @shell.exit_code
+    end
+  end
 end
 
-# If we are not running interactively, evaluate standard input
-::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(STDIN) } unless interactive
-
 if interactive
   # Output a banner message that tells users where to go for help
   @shell.print_banner
-
-  require 'irb'
-  require 'irb/ext/change-ws'
-  require 'irb/hirb'
-
-  module IRB
-    # Override of the default IRB.start
-    def self.start(ap_path = nil)
-      $0 = File.basename(ap_path, '.rb') if ap_path
-
-      IRB.setup(ap_path)
-      IRB.conf[:PROMPT][:CUSTOM] = {
-        :PROMPT_I => "%N:%03n:%i> ",
-        :PROMPT_S => "%N:%03n:%i%l ",
-        :PROMPT_C => "%N:%03n:%i* ",
-        :RETURN => "=> %s\n"
-      }
-
-      @CONF[:IRB_NAME] = 'hbase'
-      @CONF[:AP_NAME] = 'hbase'
-      @CONF[:PROMPT_MODE] = :CUSTOM
-      @CONF[:BACK_TRACE_LIMIT] = 0 unless $fullBackTrace
-
-      hirb = if @CONF[:SCRIPT]
-               HIRB.new(nil, @CONF[:SCRIPT])
-             else
-               HIRB.new
-             end
-
-      shl = TOPLEVEL_BINDING.receiver.instance_variable_get :'@shell'
-      hirb.context.change_workspace shl.get_workspace
-
-      @CONF[:IRB_RC].call(hirb.context) if @CONF[:IRB_RC]
-      # Storing our current HBase IRB Context as the main context is imperative for several reasons,
-      # including auto-completion.
-      @CONF[:MAIN_CONTEXT] = hirb.context
-
-      catch(:IRB_EXIT) do
-        hirb.eval_input
-      end
-    end
-  end
-
-  IRB.start
+end
+IRB::HIRB.new(workspace).run
+unless interactive or @shell.exit_code.nil?

Review comment:
       Not something you changed in this PR, but I wonder if `exit_code` is ever `nil?`. Seems like, if so, that'd a bug.




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] joshelser commented on a change in pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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



##########
File path: hbase-shell/src/main/ruby/jar-bootstrap.rb
##########
@@ -183,61 +183,47 @@ def debug?
 # instance variables (@hbase and @shell) onto Ruby's top-level receiver object known as "main".
 @shell.export_all(self) if top_level_definitions
 
+require 'irb'
+require 'irb/ext/change-ws'
+require 'irb/hirb'
+
+# Configure IRB
+IRB.setup(nil)
+IRB.conf[:PROMPT][:CUSTOM] = {
+  :PROMPT_I => "%N:%03n:%i> ",
+  :PROMPT_S => "%N:%03n:%i%l ",
+  :PROMPT_C => "%N:%03n:%i* ",
+  :RETURN => "=> %s\n"
+}
+
+IRB.conf[:IRB_NAME] = 'hbase'
+IRB.conf[:AP_NAME] = 'hbase'
+IRB.conf[:PROMPT_MODE] = :CUSTOM
+IRB.conf[:BACK_TRACE_LIMIT] = 0 unless $fullBackTrace
+
+# Create a workspace we'll use across sessions.
+workspace = @shell.get_workspace
+
 # If script2run, try running it.  If we're in interactive mode, will go on to run the shell unless
 # script calls 'exit' or 'exit 0' or 'exit errcode'.
-require 'shell/hbase_loader'
 if script2run
-  ::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(Hbase::Loader.file_for_load(script2run), filename = script2run) }
+  ::Shell::Shell.exception_handler(!$fullBackTrace) do
+    IRB::HIRB.new(workspace, IRB::HBaseLoader.file_for_load(script2run)).run
+  end
+  unless @shell.exit_code.nil?
+    if interactive
+      exit
+    else
+      exit @shell.exit_code
+    end
+  end
 end
 
-# If we are not running interactively, evaluate standard input
-::Shell::Shell.exception_handler(!$fullBackTrace) { @shell.eval_io(STDIN) } unless interactive
-
 if interactive
   # Output a banner message that tells users where to go for help
   @shell.print_banner
-
-  require 'irb'
-  require 'irb/ext/change-ws'
-  require 'irb/hirb'
-
-  module IRB
-    # Override of the default IRB.start
-    def self.start(ap_path = nil)
-      $0 = File.basename(ap_path, '.rb') if ap_path
-
-      IRB.setup(ap_path)
-      IRB.conf[:PROMPT][:CUSTOM] = {
-        :PROMPT_I => "%N:%03n:%i> ",
-        :PROMPT_S => "%N:%03n:%i%l ",
-        :PROMPT_C => "%N:%03n:%i* ",
-        :RETURN => "=> %s\n"
-      }
-
-      @CONF[:IRB_NAME] = 'hbase'
-      @CONF[:AP_NAME] = 'hbase'
-      @CONF[:PROMPT_MODE] = :CUSTOM
-      @CONF[:BACK_TRACE_LIMIT] = 0 unless $fullBackTrace
-
-      hirb = if @CONF[:SCRIPT]
-               HIRB.new(nil, @CONF[:SCRIPT])
-             else
-               HIRB.new
-             end
-
-      shl = TOPLEVEL_BINDING.receiver.instance_variable_get :'@shell'
-      hirb.context.change_workspace shl.get_workspace
-
-      @CONF[:IRB_RC].call(hirb.context) if @CONF[:IRB_RC]
-      # Storing our current HBase IRB Context as the main context is imperative for several reasons,
-      # including auto-completion.
-      @CONF[:MAIN_CONTEXT] = hirb.context
-
-      catch(:IRB_EXIT) do
-        hirb.eval_input
-      end
-    end
-  end
-
-  IRB.start
+end
+IRB::HIRB.new(workspace).run
+unless interactive or @shell.exit_code.nil?

Review comment:
       Not something you changed in this PR, but I wonder if `exit_code` is ever `nil?`. Seems like, if so, that'd a bug.




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] busbey closed pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

Posted by GitBox <gi...@apache.org>.
busbey closed pull request #3901:
URL: https://github.com/apache/hbase/pull/3901


   


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 33s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.4 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 34s |  branch-2.4 passed  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  branch-2.4 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  4s |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   6m 58s |  hbase-shell in the patch passed.  |
   |  |   |  17m 51s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3901 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux 4b1b1de3b37f 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 | branch-2.4 / 02fa0903a2 |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/2/testReport/ |
   | Max. process+thread count | 2672 (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-3901/2/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 36s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.4 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 47s |  branch-2.4 passed  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  branch-2.4 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 27s |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   7m  1s |  hbase-shell in the patch passed.  |
   |  |   |  18m 39s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3901 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux 2dcf7dbd6435 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2.4 / b9b075f21a |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/1/testReport/ |
   | Max. process+thread count | 2662 (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-3901/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] ndimiduk commented on a change in pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -302,33 +315,13 @@ def get_workspace
       # Install all the hbase commands, constants, and instance variables @shell and @hbase. This
       # will override names that conflict with IRB methods like "help".
       export_all(hbase_receiver)
+      # make it so calling exit will hit our pass-through rather than going directly to IRB
+      hbase_receiver.send :define_singleton_method, :exit, lambda { |rc = 0|

Review comment:
       Why must this be done via runtime injection instead of defining a method on the `HBaseReceiver` class?




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] madrob commented on a change in pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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



##########
File path: hbase-shell/src/main/ruby/shell.rb
##########
@@ -302,33 +315,13 @@ def get_workspace
       # Install all the hbase commands, constants, and instance variables @shell and @hbase. This
       # will override names that conflict with IRB methods like "help".
       export_all(hbase_receiver)
+      # make it so calling exit will hit our pass-through rather than going directly to IRB
+      hbase_receiver.send :define_singleton_method, :exit, lambda { |rc = 0|

Review comment:
       Presumably because we don't have a handle to `@shell` in the receiver




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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] busbey commented on pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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


   Changing around how we interact with and extend IRB to expressly make use of the IRB sessions feature also cleans up some sharp looking bits, which I like.
   
   The error codes look basically correct to me. I'm not sure if the `-n` flag for the "read from stdin" case should result in a non-zero exit code. I think it should?


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

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 36s |  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.  |
   ||| _ branch-2.4 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 54s |  branch-2.4 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 26s |  the patch passed  |
   | -0 :warning: |  rubocop  |   0m 15s |  The patch generated 14 new + 57 unchanged - 15 fixed = 71 total (was 72)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  10m 56s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3901 |
   | Optional Tests | dupname asflicense javac rubocop |
   | uname | Linux 716adf45331d 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 | branch-2.4 / b9b075f21a |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/1/artifact/yetus-general-check/output/diff-patch-rubocop.txt |
   | Max. process+thread count | 78 (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-3901/1/console |
   | versions | git=2.17.1 maven=3.6.3 rubocop=0.80.0 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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



[GitHub] [hbase] Apache-HBase commented on pull request #3901: HBASE-26469 WIP use IRB sessions instead of trying to make it ourselves

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


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   4m  1s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  6s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2.4 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 45s |  branch-2.4 passed  |
   | +1 :green_heart: |  javadoc  |   0m 17s |  branch-2.4 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 26s |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   7m 13s |  hbase-shell in the patch passed.  |
   |  |   |  20m  7s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/1/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3901 |
   | Optional Tests | javac javadoc unit |
   | uname | Linux ce3f4abea843 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 | branch-2.4 / b9b075f21a |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3901/1/testReport/ |
   | Max. process+thread count | 2651 (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-3901/1/console |
   | versions | git=2.17.1 maven=3.6.3 |
   | Powered by | Apache Yetus 0.12.0 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

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