Migrate Kudu to use BaseConnectorTest#11113
Conversation
This is a preparatory commit for migrating to BaseConnectorTest.
Merge TestKuduDistributedQueries into AbstractKuduConnectorTest.
ebyhr
force-pushed
the
ebi/kudu-migrate-test
branch
from
26ca440 to
79fe1c9
Compare
|
CI #11130 |
|
|
||
| <dependency> | ||
| <groupId>io.airlift</groupId> | ||
| <artifactId>log-manager</artifactId> |
There was a problem hiding this comment.
Just curious what is the log-manager dep for? I assumed it would be in test scope only.
| .build(); | ||
| } | ||
|
|
||
| public static void main(String[] args) |
There was a problem hiding this comment.
This is just for manual testing correct?
There was a problem hiding this comment.
Yes. Also this is where log-manager gets pulled in because of Logging.initialize.
grantatspothero
left a comment
grantatspothero
left a comment
There was a problem hiding this comment.
LGTM, just had a few questions
| testColumnName(columnName, true); | ||
| } | ||
|
|
||
| private void testColumnName(String columnName, boolean delimited) |
There was a problem hiding this comment.
This is same as the definition inherited from AbstractTestDistributedQueries except for the CREATE TABLE query.
Can we make the method in ATDQ protected and override here with a comment so that we know to unify this/make base class more flexible at some point in future?
There was a problem hiding this comment.
This is a general deficiency in our base tests today - they cannot be adapted to fit different CREATE TABLE/CTAS syntax. The connectors which might benefit from this are ClickHouse, Druid, Kudu and Phoenix IIRC.
There was a problem hiding this comment.
I think we can make it protected or extract one line for creating a table. Let me handle in another PR because other tests also has the similar issue.
3 participants
Description
Migrate Kudu to use BaseConnectorTest
Related issues, pull requests, and links
Documentation
(x) No documentation is needed.
Release notes
(x) No release notes entries required.