wuchong commented on pull request #12146:
URL: https://github.com/apache/flink/pull/12146#issuecomment-635064897


   Thanks for the contribution @Tartarus0zm . I ajusted the title to the 
corresponding JIRA issue. 
   
   Regarding to the fix, I agree with @rmetzger , we should use 
`HBaseConfigurationUtil.serializeConfiguration()` to serialize the conf, 
because there is not only the zk information in it. You can refer to 
`HBaseLookupFunction` about how to do it. 
   
   Besides, `HBaseRowDataInputFormat` is copied from `HBaseRowInputFormat` 
which has the same problem. It would be nice if you can fix it too. 
   
   Another thing is about the testing, the current IT case can pass is because 
it adds the hbase site xml to the classloader which is not correct in a real 
environment. I think we should remove 
[`registerHBaseMiniClusterInClasspath`](https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-hbase/src/test/java/org/apache/flink/connector/hbase/util/HBaseTestingClusterAutoStarter.java#L181).
   
   A tip: tests in flink-connector-hbase module can't run in IDEA because of 
class conflicts, but can run using mvn command. You can use `mvn test 
-Dtest=org.apache.flink.connector.hbase.HBaseConnectorITCase 
-Dcheckstyle.skip=true` in `flink-connector-hbase` directory to run individual 
tests (please mvn install the project before running this command). 
   
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to