DRILL-4241: Add Kudu reader #314
Conversation
|
+1 As long as this doesn't break the build, I say we go ahead and merge it. We can add tests and fixes later. |
|
I agree with Steven, at the hackathon we already did some basic performance and correctness verification at reasonable scale. +1 |
- Also move to a test bootstrap - Update to the latest kudu and Drill - Add plugin to Drill distribution - Checkstyle and directory cleanup
|
How could this have been merged? There is a huge double standard going on here. This code has NO comments. No tests. No documentation. No design. It isn't nearly good enough to pass the reviews that are required for others to contribute code. How can it be merged without any kind of significant review? |
|
The Kudu plugin was contributed by six different developers, three of which are Drill PMC members and two more which are PMC members of other Apache projects. The code remained available for five days for review and received two plus ones and no negative feedback. It is modeled after the HBase plugin and works the same. It is unfortunate that there aren't integrated tests (due to the fact that there wasn't an easy way to provide integrated tests such as mini hbase cluster) but it was and is regularly manually tested. Due to the light testing, we are communicating it as experimental to users. Suggesting it didn't go through review when you have that large a group of developers involved is weird. Assuming that the user api isn't in dispute (which no one here disputed most likely because Kudu looks exactly like an Oracle table), providing experimental plugins increases the breadth of Drill's appeal and thus broadens and strengthens the community. |
6 participants
Implements an experimental Kudu reader & writer.