-
Notifications
You must be signed in to change notification settings - Fork 2.8k
HBase Shell Interpreter #278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Unit tests passed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if hbase.home hbase.ruby.sources are required for this interpreter would it make sense to have explicit checks for the hbase shell script file here?
That would make the error (in the case it is not presence) more explicit and more clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already have a check. Refer to line 80.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, I'd suggest adding a check in code, not just logging the value - generally that is done in interpret() method instead of as one could return the error response to display to the user there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. Completely missing the point here. I check if there is no hbase directory and I am throwing an InterpreterException. I am doing more than just logging the value. Can you point to examples ? Isnt it better to throw the exception in open method if the parameter is wrong ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok got it, sorry I think I was looking at line 84.
Could you use something like Paths or FilenameUtils instead of joining strings and requiring one has the path separator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
On Mon, Jan 11, 2016 at 6:45 AM Felix Cheung notifications@github.com
wrote:
In hbase/src/main/java/org/apache/zeppelin/hbase/HbaseInterpreter.java
#278 (comment)
:
- }
- public HbaseInterpreter(Properties property) {
- super(property);
- }
- @OverRide
- public void open() {
- String hbase_home = getProperty("hbase.home");
- String ruby_src = getProperty("hbase.ruby.sources");
- String abs_ruby_src = hbase_home + ruby_src;
- logger.info("Home:" + hbase_home);
- logger.info("Ruby Src:" + ruby_src);
- Properties props = System.getProperties();
ok got it, sorry I think I was looking at line 84.
Could you use something like Paths
http://docs.oracle.com/javase/7/docs/api/java/nio/file/Paths.html or
FilenameUtils
https://commons.apache.org/proper/commons-io/javadocs/api-1.4/org/apache/commons/io/FilenameUtils.html
instead of joining strings and requiring one has the path separator?—
Reply to this email directly or view it on GitHub
https://github.com/apache/incubator-zeppelin/pull/278/files#r49285311.
|
Could you elaborate on what this does? |
|
@felixcheung I have addressed your comments. Please take a look |
|
The failure is a false positive. I made only cosmetic changes. Looks like it failed in downloading npm stuff. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should rethrow the exception too? I assume the interpreter will not work if this fails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block is used for testing only. I'll add a comment.
|
@vrajat do you plan to update this PR\add some docs for new interpreter? |
|
I'll put some effort into clean up the PR.
|
|
@felixcheung Done |
|
thanks, trying to test this now. |
|
also, I realize when testing this, you should add to the list in ZeppelinConfiguration.java as documented in here. |
|
@vrajat I am having problem getting this to work. It's likely this is my setup but I wonder if you have any idea to fix this
When I ran this I got this and in the log |
|
Surprising. It seems like the hbase ruby scripts arent loaded. I used the On Thu, Jan 21, 2016 at 11:32 AM Felix Cheung notifications@github.com
|
|
|
|
@vrajat I saw a documentation Plus, a few days ago, there is a PR #648 related to fixing documentation styles. This PR was working on deleting a numbering in the each sections. So why don't you remove a numbering? Thank You ! : ) |
|
@vrajat ? |
|
Sorry. I've been busy with work. I'll try to get it next week. I'll look at
|
|
Sure - let's get it working, and here. I can help with updating doc if necessary. |
|
Ah ha! Now I remember why I had my own copy of hirb.rb. I couldnt get this piece of code in the original hirb.rb to work. So I had deleted it and it had no bad consequences. |
|
@vrajat I see this error when I run it But the path seems correct: Also there is a typo in zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be org.apache.zeppelin.hbase.HbaseInterpreter
|
@felixcheung I've got past that error. I am hitting This usually implies a mismatch between the jars used in the hbase client and the hbase server. I am looking into it. The same code & dependencies works in the Qubole fork. So I am eliminating what could be different about this branch. |
|
cool, let me know when I should try again. |
|
@felixcheung Please take a look now. I have fixed all bugs and I can use it on my HBase setup (HBase 1.0.0 and Hadoop 2.6.0) |
|
Great! It works (tested with hbase-1.0.2). |
|
w00t! On Tue, Feb 2, 2016 at 11:25 AM Felix Cheung notifications@github.com
|
|
Cool! |
|
@felixcheung hello, i saw you using hbase-1.0.2 works fine, but i using hbase 2.4.9 got the same error: INFO [2022-05-24 14:20:49,513] ({FIFOScheduler-org.apache.zeppelin.hbase.HbaseInterpreter679627819-Worker-1} HbaseInterpreter.java[open]:80) - Home:/data/ygh/hbase-2.4.9 Below are the verisons : Apache Zeppelin - 0.10.1 do you have any idea to fix this? |

Support for Hbase Shell. All the commands documented here https://wiki.apache.org/hadoop/Hbase/Shell is supported.
Requirements:
Hbase Shell should be installed on the same machine. To be more specific, the following dir. should be available: https://github.com/apache/hbase/tree/master/hbase-shell/src/main/ruby
Hbase Shell should be able to connect to the Hbase cluster from terminal. This makes sure that the client is configured properly.
The interpreter takes 3 config parameters:
hbase.home: Root dir. where hbase is installed. Default is /usr/lib/hbase/
hbase.ruby.sources: Dir where shell ruby code is installed. Path is relative to hbase.home. Default: lib/ruby
hbase.irb.load: (Testing only) Default is true. Whether to load irb in the interpreter.