-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[Connector]Add hbase source connector #6348
Conversation
...l-e2e/seatunnel-connector-v2-e2e/connector-hbase-e2e/src/test/resources/hbase-to-assert.conf
Show resolved
Hide resolved
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.
update plugin-mapping.properties?
...src/main/java/org/apache/seatunnel/connectors/seatunnel/hbase/source/HbaseSourceFactory.java
Show resolved
Hide resolved
|
Excuse me, is there any support about Kerberos authentication in hbase source connector? |
|
I have modified the code according to the suggestion, thanks for @hailin0 @Hisoka-X review. I have communicated with @lihjChina on wechat. At present, he has used this pr in the production environment of the company. After this pr is merged, next he will contribute to add Kerberos authentication in hbase source connector. |
|
cc @TyrantLucifer as well. |
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.
LGTM
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.
Overall LGTM, Could you please offer some screeshots to verify it worked? Maybe require three engines(flink/spark/zeta).
There are also relevant screenshots of the data that need to be verified for correct data reading.
Due to hbase e2e did not be enabled in CI/CD, so there has no way to prove that this connector is usable.
| Connection connection = ConnectionFactory.createConnection(hbaseConfiguration); | ||
| return connection; | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); |
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.
use connector exception
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.
| return Double.valueOf(Bytes.toString(cell)); | ||
| case BYTES: | ||
| return cell; | ||
| case DATE: |
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.
date/time/timestamp as String? It maybe has some problem. We can add parameters to setting the format of these types as text file connector.
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.
date/time/timestamp as String? It maybe has some problem. We can add parameters to setting the format of these types as text file connector.
+1
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.
Thanks for advice. I have already modified it.
|
Why not use hbase docker? |
| |--------------------|--------|----------|---------------| | ||
| | zookeeper_quorum | string | yes | - | | ||
| | table | string | yes | - | | ||
| | query_columns | list | yes | - | |
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.
From this option we can implement schema projection ? If true, You need update here
[x] [schema projection](../../concept/connector-v2-features.md)
and the HbaseSource need implement SupportColumnProjection interface.
HbaseSource need implement SupportParallelism too, because it support parallelism .
|
I found that the |
|
Please fix ci |







Purpose of this pull request
#3018
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide
release-note.