Simple HBase Log Appender implementation #1535
Conversation
|
The corresponding Jira Issue is: |
There was a problem hiding this comment.
Hi, @JoaoRei! Thanks for the patch!
- The build failure is not related to your changes and is safe to ignore.
- Please check the code style. It is inconsistent (mixed tabs and spaces, wrong indentation levels, etc.) You can find more info here: http://kaaproject.github.io/kaa/docs/v0.10.0/How-to-contribute/Code-style/Java/
|
Hi @rasendubi! I made the changes in the code style to match the Google Java Style as it is stated in the documentation link you sent me. |
| <dependency> | ||
| <groupId>org.apache.hbase</groupId> | ||
| <artifactId>hbase-common</artifactId> | ||
| <version>0.98.6-hadoop2</version> |
There was a problem hiding this comment.
Here and below please use Dependency Management to specify dependencies versions. See example in the
appenders.
| <exclude>**/logback.xml</exclude> | ||
| </excludes> | ||
| </configuration> | ||
| </plugin> --> |
There was a problem hiding this comment.
Uncomment this configuration.
| <plugin> | ||
| <groupId>org.apache.avro</groupId> | ||
| <artifactId>avro-maven-plugin</artifactId> | ||
| <version>1.7.7</version> |
There was a problem hiding this comment.
We are commonly using the Avro version specified here.
Let's use <version>${avro.version}</version> instead of <version>1.7.7</version>.
| "DIFF", | ||
| "FAST_DIFF", | ||
| "PREFIX", | ||
| "PREFIX_TREE" |
There was a problem hiding this comment.
Could you apply vertical alignment to this file?
|
|
||
| LOG.info("HBase Admin Started"); | ||
| } catch (Exception e) { | ||
| LOG.error("Ops!", e); |
There was a problem hiding this comment.
Better to provide the more detailed error message. For example, as you do it here.
Also, it's not allowed by our check style to name variable with only one letter, replace e with ex.
| //Get table and keyspace names | ||
| String table = configuration.getTableName().toString().toLowerCase().trim(); | ||
| String keyspace = configuration.getKeyspace().toString().toLowerCase().trim(); | ||
| LOG.info("Starting creation of table: {}",keyspace + ":" + table); |
There was a problem hiding this comment.
Add the space before keyspace.
| namespacedTable = keyspace + ":" + table; | ||
|
|
||
| } catch (Exception e) { | ||
| LOG.error("Ops",e); |
There was a problem hiding this comment.
See related comment above.
|
Hi, @JoaoRei! See existed log appenders doc and its implementation. |
|
@JoaoRei I don't want this PR to rot. I see the comments are non-critical, so I'll merge it now. Let's make it more iterative and address the comments in a separate PR if you have time and will ;) |
|
@alexey Shmalko
Hello!
Thank you very much for your support and feedback!
I am willing to continue working on this contribution by correcting the
pending issues and providing a documentation for the log appender
implementation (:
I am sorry for the late response, but i didn't have the required time to
correct them since I am writing my master thesis. It is almost finished and
in the next week I will have plenty of time to get back to work on this
project! I will try to address the issues in a new separate PR soon!
Once again, thank you very much for your understanding and support along
the process (:
Best regards,
João Rei
2018-04-04 21:37 GMT+01:00 Alexey Shmalko <notifications@github.com>:
… Merged #1535 <#1535>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1535 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/Aj5csqU_Bf_mBrmRjZJDrlYzHuLSuUgXks5tlS8lgaJpZM4S5h0A>
.
|
Hey,
This is a simple HBase Log Appender implementation, based on Cassandra Log Appender.
It was developed with HBase-0.98.6 and it was used in my master thesis project.
It provides a simple way to integrate Kaa in an Hadoop Distributed File System (HDFS) through HBase database.