HBASE-11686 Shell code should create a binding / irb workspace instead of polluting the root namespace #2141
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d of polluting the root namespace - Refactor Shell.export_commands to define commands using ruby lambdas. Additionally, this change stores a reference to shell_inst in scope so that we no longer need to assume the existance of the variable @shell. - Add logic to Shell class for constructing an IRB workspace with its own binding and non-global receiver. This workspace is loaded with all HBase and IRB commands. - Create new method on Shell for evaluating input from an arbitrary IO instance within the created IRB workspace. This is based on work by Hsieh that was previously in bin/hirb.rb. This method is more generic and more testable. This single pattern can be used for both executing files and reading from stdin, therefore reducing complexity. - Move special 'help' and 'tools' command definitions to shell.rb. These commands are tightly linked with an instance of the shell, so it is easiest to have them defined together. - Remove all global includes of HBaseConstants from ruby test files. Before this change, tests were loading these constants into the top level, which could cause tests to pass that should really fail. - Try to reduce the number of places that constants are included. I think it's best to reference each ruby constant's full name, but where that would cause a big diff I instead moved the include to the innermost Module or Class. - Update docs and comments - Remove unneccessary includes - Add shell --top-level-cmds compatibility flag. Since this PR removes all the HBase symbols from the top-level receiver (ie. main Object), it is possible (albeit unlikely) that this will break operator scripts. This flag will export all the commands at the top-level like the shell previously did.
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
- Fixes some constants references by admin test 2 - Install HBase commands as singleton methods on recevier instances so that multiple receivers may exist. - Rename new flag from --top-level-cmds to --top-level-defs to be more semantically accurate. - Create new helper method Shell::Shell#export_all to install @hbase, @shell, constants, and all hbase commands to a target receiver. As a result, the HBaseReceiver became very simple and could be moved to shell.rb. - Add unit tests for Shell::Shell#eval_io and Shell::Shell#export_all - Add @hbase and @shell to hbase-shell IRB workspace - Fix robocop issues within patch
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
saintstack
approved these changes
Jul 27, 2020
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Failures are in the abandoned backup module. |
asfgit
pushed a commit
that referenced
this pull request
Oct 14, 2020
…d of polluting the root namespace (#2141) * HBASE-11686 Shell code should create a binding / irb workspace instead of polluting the root namespace - Refactor Shell.export_commands to define commands using ruby lambdas. Additionally, this change stores a reference to shell_inst in scope so that we no longer need to assume the existance of the variable @shell. - Add logic to Shell class for constructing an IRB workspace with its own binding and non-global receiver. This workspace is loaded with all HBase and IRB commands. - Create new method on Shell for evaluating input from an arbitrary IO instance within the created IRB workspace. This is based on work by Hsieh that was previously in bin/hirb.rb. This method is more generic and more testable. This single pattern can be used for both executing files and reading from stdin, therefore reducing complexity. - Move special 'help' and 'tools' command definitions to shell.rb. These commands are tightly linked with an instance of the shell, so it is easiest to have them defined together. - Remove all global includes of HBaseConstants from ruby test files. Before this change, tests were loading these constants into the top level, which could cause tests to pass that should really fail. - Try to reduce the number of places that constants are included. I think it's best to reference each ruby constant's full name, but where that would cause a big diff I instead moved the include to the innermost Module or Class. - Update docs and comments - Remove unneccessary includes - Add shell --top-level-cmds compatibility flag. Since this PR removes all the HBase symbols from the top-level receiver (ie. main Object), it is possible (albeit unlikely) that this will break operator scripts. This flag will export all the commands at the top-level like the shell previously did. * HBASE-11686 Light refactoring with added unit tests - Fixes some constants references by admin test 2 - Install HBase commands as singleton methods on recevier instances so that multiple receivers may exist. - Rename new flag from --top-level-cmds to --top-level-defs to be more semantically accurate. - Create new helper method Shell::Shell#export_all to install @hbase, @shell, constants, and all hbase commands to a target receiver. As a result, the HBaseReceiver became very simple and could be moved to shell.rb. - Add unit tests for Shell::Shell#eval_io and Shell::Shell#export_all - Add @hbase and @shell to hbase-shell IRB workspace - Fix robocop issues within patch * Typo s/is/if/ (cherry picked from commit 7eff07d)
clarax
pushed a commit
to clarax/hbase
that referenced
this pull request
Nov 15, 2020
…d of polluting the root namespace (apache#2141) * HBASE-11686 Shell code should create a binding / irb workspace instead of polluting the root namespace - Refactor Shell.export_commands to define commands using ruby lambdas. Additionally, this change stores a reference to shell_inst in scope so that we no longer need to assume the existance of the variable @shell. - Add logic to Shell class for constructing an IRB workspace with its own binding and non-global receiver. This workspace is loaded with all HBase and IRB commands. - Create new method on Shell for evaluating input from an arbitrary IO instance within the created IRB workspace. This is based on work by Hsieh that was previously in bin/hirb.rb. This method is more generic and more testable. This single pattern can be used for both executing files and reading from stdin, therefore reducing complexity. - Move special 'help' and 'tools' command definitions to shell.rb. These commands are tightly linked with an instance of the shell, so it is easiest to have them defined together. - Remove all global includes of HBaseConstants from ruby test files. Before this change, tests were loading these constants into the top level, which could cause tests to pass that should really fail. - Try to reduce the number of places that constants are included. I think it's best to reference each ruby constant's full name, but where that would cause a big diff I instead moved the include to the innermost Module or Class. - Update docs and comments - Remove unneccessary includes - Add shell --top-level-cmds compatibility flag. Since this PR removes all the HBase symbols from the top-level receiver (ie. main Object), it is possible (albeit unlikely) that this will break operator scripts. This flag will export all the commands at the top-level like the shell previously did. * HBASE-11686 Light refactoring with added unit tests - Fixes some constants references by admin test 2 - Install HBase commands as singleton methods on recevier instances so that multiple receivers may exist. - Rename new flag from --top-level-cmds to --top-level-defs to be more semantically accurate. - Create new helper method Shell::Shell#export_all to install @hbase, @shell, constants, and all hbase commands to a target receiver. As a result, the HBaseReceiver became very simple and could be moved to shell.rb. - Add unit tests for Shell::Shell#eval_io and Shell::Shell#export_all - Add @hbase and @shell to hbase-shell IRB workspace - Fix robocop issues within patch * Typo s/is/if/ (cherry picked from commit 7eff07d)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves https://issues.apache.org/jira/browse/HBASE-11686
Goal
Stop defining HBase commands and constants on Ruby's main object. Instead, HBase commands and constants are defined within the scope of an Interactive Ruby (IRB) Workspace.
Consequences for Scripts
When using script via STDIN (
echo version | hbase shell) or filename (hbase shell script.rb), the code is evaluated by this PR within the created IRB Workspace. Code should work as before unless the script "breaks out" of the current binding (for example, this could be done using Ruby'sTOPLEVEL_BINDING). For cases like this one, I added a flag--top-level-defsthat should be backwards compatible with all scripts.Changelog
using ruby lambdas. Additionally, this change stores a reference to shell_inst
in scope so that we no longer need to assume the existance of the variable
@shell.
binding and non-global receiver. This workspace is loaded with all HBase and
IRB commands.
within the created IRB workspace. This is based on work by Hsieh that was
previously in bin/hirb.rb. This method is more generic and more testable.
This single pattern can be used for both executing files and reading from
stdin, therefore reducing complexity.
commands are tightly linked with an instance of the shell, so it is easiest
to have them defined together.
this change, tests were loading these constants into the top level, which
could cause tests to pass that should really fail.
best to reference each ruby constant's full name, but where that would cause
a big diff I instead moved the include to the innermost Module or Class.
HBase symbols from the top-level receiver (ie. main Object), it is possible
(albeit unlikely) that this will break operator scripts. This flag will
export all the commands at the top-level like the shell previously did.
constants, and all hbase commands to a target receiver. As a result, the
HBaseReceiver became very simple and could be moved to shell.rb.